windows memory formatting issue

Question

I am trying to make this dynamic reallocation work in a portable fashion.

My program accepts a line of text from a user and appends it to a buffer. If the length of text in the buffer is 20 or more, it removes the first 20 characters and moves any characters after that to the start of the buffer.

I have this code which works clean on Linux but when I run it on windows it emits garbage. Does anyone know why/how to make this portable only using malloc. IE not using string.h(strcpy) str... anything but len.

c17 only - no broken stucts(not portable). here is my code. Compiles with no errors gcc 7.3, mingw 7.3. I replace gets and puts with safer functions and I still get garbage on windows. I assume this is a formatting issue...

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <malloc.h>

void wbuff (message)
    char *message;
{
    FILE *f = fopen("file.txt", "w");
    fprintf(f, "%s", message);
    fclose(f);
}

char *rean (message)
    char *message;
{
    /* performs (write) on buffer, trims lefover, then restores */

    char buf[80] = "";
    puts("enter a line");
    gets(buf);

    int bln  =  strlen( buf );
    int mln  =  strlen( message );
    int nln  =  bln + mln;
    printf("new length %d\n", nln);

    message = realloc(message, nln);
    memmove(message + mln, buf, bln);

    /* MISTAKE IS HERE?! */
    if( nln >= 20 ) {
        int exl  = nln -20;                        // leftover length
        char *lo = realloc(NULL, exl);             // leftover placeholder
        memmove(lo, message+20, exl);              // copy leftover
        wbuff(message);                            // write clear buff
        message = realloc(NULL, nln);
        message = realloc(NULL, exl);              // resize buffer
        memmove(message, lo, exl);                 // restore leftover
    }

    return message;
}

void main (void)
{
    char *message = "";
    message = realloc(NULL, 0);
    while ( 1 == 1 ) {
        message = rean( message );
        puts(message);
    }
    return;
}

Answer 1

In C, strings are a sequence of characters terminated by a null byte. You have a number of off-by-one errors here mostly related to not accounting for that fact, as well as memory leaks.

When you first set message in main:

char *message = "";
message = realloc(NULL, 0);

message either is NULL to points to 0 bytes of memory. When you call then call rean the first time:

int mln  =  strlen( message );

You're either attempting to dereference a NULL pointer to read past the end of allocated memory. You want to allocate at least 1 byte to start and set that byte to 0 so you have an empty string:

char *message = realloc(NULL, 1);
message[0] = '\0';

Then later when you copy the buffer into the message:

message = realloc(message, nln);
memmove(message + mln, buf, bln);

You don't allocate enough space for the terminating null byte, nor do you copy it, so you don't really have a string. When you then try to print it, either puts or printf reads past the end of the allocated memory. You need to allocate 1 extra byte and copy 1 extra byte:

message = realloc(message, nln + 1);     // allocate 1 extra byte for the null terminator
memmove(message + mln, buf, bln + 1);    // copy 1 extra byte

There are similar issues when you recopy anything past 20 characters:

 int exl  = nln -20;                        // leftover length
 char *lo = realloc(NULL, exl);             // leftover placeholder
 memmove(lo, message+20, exl);              // copy leftover
 wbuff(message);                            // write clear buff
 message = realloc(NULL, nln);
 message = realloc(NULL, exl);              // resize buffer
 memmove(message, lo, exl);                 // restore leftover

• lines 2-3: You don't allocate space for the terminating null byte for lo nor do you copy it.
• line 5: You leak the memory previously held by message in the first realloc by passing assigning to message while using NULL as the first argument
• line 6-7: You leak the memory allocated in line 5 by doing the same thing. Also, you again don't allocate space for the null byte, nor do you copy it on the next line.

As before, allocate 1 extra byte for each allocation and move 1 extra byte to account for the null terminator. Also, free lo at the end of the block, remove the extra realloc for message, and pass the prior value of message to realloc so you don't leak memory:

 int exl  = nln -20;                        
 char *lo = realloc(NULL, exl + 1);         // allocate 1 extra byte
 memmove(lo, message+20, exl + 1);          // copy 1 extra byte
 wbuff(message);                            
                                            // remove extra realloc
 message = realloc(message, exl + 1);       // pass in old message, allocate 1 extra
 memmove(message, lo, exl + 1);             // copy 1 extra byte
 free(lo);                                  // free leftover

These issues of reading and writing past the end of allocated memory all invoke undefined behavior, which explains why you see different results on different operating systems.

As far as conforming code goes, use fgets intead of gets:

 fgets(line, sizeof(line), stdin);

This function will include a newline in line if there's space for it, so be sure to remove it if that's the case.

Also change main to return int, and remove #include <malloc.h> since the malloc family of function is defined to reside in stdlib.h.

Had you used strcpy and strcat instead of memmove, you wouldn't have had to account for copying the null terminating byte as those functions do that for you. You would still however need to account for that when allocating memory. There's also no conflict between strcpy, malloc, and realloc, as they are all part of the standard and work together properly. Using them together is no problem. If they aren't working properly for you then you aren't using them correctly.

After applying my updates, you can replace this:

memmove(message + mln, buf, bln + 1);

With this:

strcat(message, buf);

And replace this:

 memmove(lo, message+20, exl + 1);              // copy leftover
 ...
 memmove(message, lo, exl + 1);                 // restore leftover

With this:

 strcpy(lo, message+20);
 ...
 strcpy(message, lo);

And it will still work properly and be conforming.