0

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

7
  • 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. Commented Jun 1, 2014 at 17:43
  • direntName is just a char *. That's a good idea, I'll definitely do that. Commented Jun 1, 2014 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. Commented Jun 1, 2014 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) ); Commented Jun 1, 2014 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. Commented Jun 1, 2014 at 19:46

2 Answers 2

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).

Sign up to request clarification or add additional context in comments.

1 Comment

That was it, thanks!! I had the impression the new pointer pointed to the new empty memory space at the end of the block.
1

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.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.