Problem with getpwuid() and stat()

Question

I have the following code. It gives me a problem with free memory, but I haven't figured out what exactly the problem is. It seems that getpwuid(buf->st_uid); is not getting along with readdir(dirh); or with stat functions. Does anyone know why?

buf = (struct stat*) malloc(sizeof(struct stat));        
for (dirp[i] = readdir(dirh); dirp[i] != NULL; dirp[++i] = readdir(dirh)){

    ptr=(char *)malloc(sizeof(strlen(mydir)));

    ptr=strdup(mydir);
    strcat(ptr,dirp[i]->d_name);

    stat(ptr,buf);
    //modos();

    getpwuid(buf->st_uid);
    printf("\t%s\n",ptr);

    //we free the buf memory
}

free(buf);
closedir(dirh);

Answer 1

You don't allocate enough space for the string.

You don't quite allocate enough for 'mydir' (off by one); You do an unorthodox length calculation which ignores the actual length of 'mydir'; you then make a good copy of the directory name (but leak the memory previously allocated); you then concatenate the name over the end of the space allocated by strdup(), which is always bad. If you omitted the sizeof(), then you would be allocating one byte too few for a null terminated directory string.

You also should not cache the separate return values from readdir() because it generally returns a pointer to the same bit of memory each time. You are certainly not authorized to assume that it does otherwise.

You are also trampling over freed memory if you really release buf inside the loop; you allocate just once, and free many times.

Normally, you would not bother to allocate the buf at all; it is not that big a structure.

You also don't self-evidently have a slash separator between the directory name and the file name component; that might not matter if you've ensured that mydir has one on the end.

---

Here is a simple program that more or less does what is necessary.

#include <stdio.h>
#include <dirent.h>
#include <sys/stat.h>
#include <pwd.h>
#include <string.h>
#include <stdlib.h>

int main(void)
{
    static const char mydir[] = "./";
    DIR *dirh;
    size_t dlen = strlen(mydir) + 1;

    if ((dirh = opendir(mydir)) != 0)
    {
        struct dirent *dirp;
        while ((dirp = readdir(dirh)) != 0)
        {
            char *str = malloc(dlen + strlen(dirp->d_name));
            if (str != 0)
            {
                struct stat buf;
                strcpy(str, mydir);
                strcat(str, dirp->d_name);
                if (stat(str, &buf) == 0)
                {
                    struct passwd *pwd = getpwuid(buf.st_uid);
                    if (pwd != 0)
                        printf("\t%s (%s)\n", str, pwd->pw_name);
                }
                free(str);
            }
        }
        closedir(dirh);
    }
    return 0;
}

Answer 2

ptr=(char *)malloc(sizeof(strlen(mydir)));
ptr=strdup(mydir);

looks problematic at best :-)

You're allocating some memory in the first then leaking it when you allocate more memory in the second.

Even if that weren't a problem, I've never seen the paradigm:

sizeof(strlen(mydir))

You would tend to either use sizeof or strlen()+1 when calling malloc. I think what you have, even if it worked, would simply allocate enough space for a size_t, the return value from strlen. I could be wrong on that count but I don't think it's relevant since you're not using that memory anyway due to the leak.

There are other problems such as:

• getpwuid is supposed to return a struct passwd * which you're ignoring.
• strdup gives you a copy of a string that is exactly the right size, then you strcat on to it, almost certainly corrupting memory.

Now this is untested but I think it's probably a better starting position:

struct stat buf;
struct dirent * direntp;
struct passwd * pwdp
for (direntp = readdir(dirh); direntp != NULL; direntp = readdir(dirh)) {
    // Allocate enough space for directory, separator, file and nul character.

    ptr = (char*)malloc (strlen(mydir) + 1 + strlen (direntp->d_name) + 1);

    strcpy (ptr, mydir);
    strcat (ptr,"/");
    strcat (ptr, direntp->d_name);

    stat(ptr, &buf);
    //modos();

    pwdp = getpwuid (buf.st_uid);
    printf("\t%s\n",ptr);  // prob. need something from pwdp printed here as well.

    // Don't leak.    
    free (ptr);
}

closedir(dirh);