C - Realloc invalid next size w strings

Question

I'm having a problem in C when listing the files from a folder.

The strange here is that it works fine several times, but then the program is calling other functions and after that run again then the function to list the files.

I added a print malloc_usable_size - it says that has enough space but when it breaks out it says 0.

Also when is broken out the ent->d_name has some strange characters.

At the end finishes with an error: realloc(): invalid next size

Do you have any idea?

Here is the code:

struct dirent *ent;
int size = 6;
char *file_names = NULL, *temp = NULL;
while((ent=readdir(dirp))!=NULL) {
  if( (strcmp(ent->d_name, ".")!=0) && (strcmp(ent->d_name, "..")!=0) ) {
    size += strlen(ent->d_name)*sizeof(char) + 6;
    temp = file_names;
    file_names = (char *) realloc(file_names, size);
    if(file_names != NULL) {
      strcat(file_names, ent->d_name);
      strcat(file_names, "\n\0");
    }
    else {
      file_names = temp;
    }
  }
}
closedir(dirp);
if(file_names != NULL) {
  strcat(file_names, "\0");
}

score 2 · Answer 1 · edited May 23 '17 at 11:49

2

strcat appends to the end of a string. But you never start off with a string; the first call to realloc gets you uninitialized memory. Perhaps it chances that you get a zero byte the first time, but after other functions have used and freed memory, next time you allocate memory it started with a non-zero byte.

You'll need to set file_names[0] = 0; after the first allocation. (e.g. if ( temp == NULL ) file_names[0] = 0;

BTW it's more usual to use this pattern for realloc: (and don't cast it)

temp = realloc(file_names, size);
if ( temp != NULL )
{
    if ( file_names == NULL ) 
        temp[0] = 0;

    file_names = temp;  
    strcat(file_names, ent->d_name);
    strcat(file_names, "\n");   // extra \0 is redundant 
}

NB. The algorithm is rather inefficient (every call to strcat has to scan the whole string again). You could instead store the current offset; this would also fix your problem with the initial strcat. E.g. (pseudocode)

// before loop
size_t offset = 0;

// in the loop; after allocating the right amount of memory as before
offset += sprintf(file_names + offset, "%s\n", ent->d_name);

edited May 23 '17 at 11:49

Community

1
1

answered Jul 01 '14 at 23:02

M.M

138,810
21
208
365

All excellent points. I was also wondering whether `size` might overflow (-> negative, "invalid size"). I would use a `size_t` not an `int` to be defensive. – Floris Jul 01 '14 at 23:10
other programs? More like the same program reusing its memory, giving memory to a program without zeroing it first is a damn security risk. – Deduplicator Jul 01 '14 at 23:13
@Floris Seems unlikely that a dir listing would exceed INT_MAX. BUt using `size_t` would be better as you say, because it is actually a size of memory that it is storing. – M.M Jul 01 '14 at 23:13
@Deduplicator fixed, I had a brainfart with "the program is calling other functions" – M.M Jul 01 '14 at 23:14
Also, calling `printf` is really not efficient. Better a `strlen` on the string which shall be appended (may be superfluous), a memcpy + manually setting the newline and the terminator (after the loop). – Deduplicator Jul 01 '14 at 23:17
INT_MAX depends on the compiler. It might be 32k long. There is a "times six" in the allocation so a string of 5k characters could do it. As I said "defensive coding" tries to stay one step ahead. IIRC the type of the parameter in the `realloc` call is also `size_t`. – Floris Jul 01 '14 at 23:17
Agree with @deduplicator. Make it a `memcpy` So you don't keep copying characters. – Floris Jul 01 '14 at 23:19
@Floris yeah the sprintf could be further micro-optimized to a memcpy + setting `\n`, then `\0` after the loop; and cache the current length being appended in a variable. – M.M Jul 01 '14 at 23:23
1

Another inefficiency is calling `realloc` for every single filename. Usually it would be called for a reasonably-sized chunk of additional memory at a time. That requires keeping track of both the total size of the currently-allocated chunk and also the used portion. – ooga Jul 01 '14 at 23:23

C - Realloc invalid next size w strings

1 Answers1