Using realloc for array of structs

Question

I'm using realloc to increase the amount of memory for an array of structs and it doesn't seem to be increasing at all. I've read all of the posts relating to this and I can't figure out what I'm doing wrong. Here is my code:

struct fileInfo {
char name[MAXNAMLEN];
struct stat stbuf;
};

...

 struct fileInfo * newFileInfoPtr;
 struct fileInfo * fileInfoPtr = (struct fileInfo *) realloc (NULL, sizeof (struct fileInfo));
    // loop through directory to find all filenames
    while ((direntPtr = readdir (dirFilePtr)) != NULL) {
        (void) strcpy (direntName, filename);
        (void) strcat (direntName, "/");
        (void) strcat (direntName, direntPtr->d_name);
        (void) strcat (direntName, "\0");
        // check for vaild file
        if (lstat (direntName, &st)) {
            if (errno) {
                (void) snprintf (buffer, BUFSIZ, STR_LSTAT_ERR);
                perror (buffer);
            }
            exit (EXIT_FAILURE);
        }

        // reallocate memory for new fileInfo struct
        newFileInfoPtr = (struct fileInfo *) realloc (fileInfoPtr, 
                                    (count+1) * sizeof (struct fileInfo));

        // check for allocation success and increase count
        if (newFileInfoPtr != NULL) {
            (void) memset (newFileInfoPtr->name, NULL, MAXNAMLEN);
            (void) strcpy (newFileInfoPtr->name, direntPtr->d_name);
            newFileInfoPtr->stbuf = st; 
            fileInfoPtr = newFileInfoPtr;
            count++;
        } else {
            if (errno) {
                (void) snprintf (buffer, BUFSIZ, STR_ALLOC_ERR);
                perror (buffer);
            }
            exit (EXIT_FAILURE);
        }
        ...

direntPtr is a pointer to an opened directory and I am reading all the names of the files within. That part is working because I can print out the names of each entry. My problem is the final struct array I end up with. I understand with realloc you allocate memory based on the old array, assign the return value to a new pointer, fill the new pointer with information, and set the old pointer to the new pointer. However, after finishing the loop, fileInfoPtr is just one struct. I can access the fields at index 0, but after that they are all empty. I printed out the memory addresses of each index and they are all the same... Thanks for any help

You don't show the variable `direntName`...what type is it? You'd probably do better with `snprintf()` than `strcpy()` plus 3 `strcat()` calls. However, that's probably tangential to your problem. — Jonathan Leffler, Jun 01 '14 at 17:43
direntName is just a char *. That's a good idea, I'll definitely do that. — Ryan McClure, Jun 01 '14 at 17:47
how is direntName defined? the order of the data into direntName is reversed. I.E. strcat appends to first parameter. using lstat(), errno is only set if an error occurs. — user3629249, Jun 01 '14 at 19:34
the line perror (buffer); is accessing an area that is not defined. it should be referencing lstat, something like perror( "lstat( direntName ) %s", strerror(errno) ); — user3629249, Jun 01 '14 at 19:42
the if statement following the //check for allocation success is putting everything into the beginning of the realloc'd buffer. It should be placing the info into &newFileInfoPtr[count*sizeof(struct fileInfo) )] . hopefully count was initialized at 0. — user3629249, Jun 01 '14 at 19:46
for various reasons, including the hiding of what is actually happening, the line: newFileInfoPtr->stbuf = st; should be: memcpy( &newFileInfoPtr[count*sizeof(strutFileInfo))]->stbuf, st, sizeof(struct fileInfo) ); — user3629249, Jun 01 '14 at 19:50
There is the bug in the code where the fileInfoPtr points to an area that one sizeof(struct fileInfo) too long. — user3629249, Jun 01 '14 at 19:52

score 2 · Accepted Answer · answered Jun 01 '14 at 17:50

2

Your newFileInfoPtr points to the first entry of the newly allocated array. With your memset and strcpy calls, you operate on newFileInfoPtr->name, which overwrites the name of the first entry. You want

(void) memset (newFileInfoPtr[count].name, NULL, MAXNAMLEN);
(void) strcpy (newFileInfoPtr[count].name, direntPtr->d_name);

to access the last entry at index count (before incrementing).

answered Jun 01 '14 at 17:50

M Oehm

28,726
3
31
42

That was it, thanks!! I had the impression the new pointer pointed to the new empty memory space at the end of the block. – Ryan McClure Jun 01 '14 at 17:53

Jonathan Leffler · Answer 2 · 2014-06-01T18:04:13.280

You're doing some things right and some things wrong with realloc().

The thing I'd pick on as correct is:

newFileInfoPtr = (struct fileInfo *) realloc(fileInfoPtr, 
                                             (count+1) * sizeof (struct fileInfo));

This carefully ensures that you don't lose allocated space if the allocation fails. However, reallocating every time is expensive; it can lead to quadratic behaviour. You should allocate twice as many entries each time, or minor variants on that idea.

However, one of the correct follow-up steps, after checking for non-null, is:

fileInfoPtr = newFileInfoPtr;

You also need to use the fileInfoPtr as an array:

strcpy(FileInfoPtr[count].name, direntPtr->d_name);

We can negotiate on whether the index is count or count ± 1; the general idea is that you need to use the array of struct fileInfo as an array, rather than always using the zeroth element of the array for everything.

Were it my code, I'd probably use a structure like:

typedef strut fileInfo FileInfo;

size_t count = 0;
size_t avail = 0;

FileInfo *fileInfoPtr = 0;
// loop through directory to find all filenames
while ((direntPtr = readdir(dirFilePtr)) != NULL)
{
    snprintf(direntName, sizeof(direntName), "%s/%", filename, direntPtr->d_name);
    if (lstat(direntName, &st))
    {
        int errnum = errno;
        snprintf(buffer, sizeof(buffer), "Failed to lstat() %s (%d: %s)\n",
                 direntName, errnum, strerror(errnum));
        perror(buffer);
        exit(EXIT_FAILURE);
    }

    if (count >= avail)
    {
         size_t new_size = (avail + 1) * 2;
         FileInfo *np = realloc(fileInfoPtr, new_size);
         if (np == 0)
         {
             …handle error; release old fileInfoPtr…
             exit(EXIT_FAILURE);
         }
         avail = new_size;
         fileInfoPtr = np;
    }

    strcpy(fileInfoPtr[count].name, direntPtr->d_name);
    InfoPtr[count].stbuf = st;
    count++;
}

At the end of the loop, count tells you how many entries are in use in the array, and avail tells you how many entries are allocated. If you're worried about too much space, you could do a final shrinking realloc() to make the array the exact size you need. It is seldom necessary to actually do so.

Note that the memset() before strcpy() is not necessary.

Using realloc for array of structs

2 Answers2

Linked