Using malloc properly

Question

I'm trying to put my directory pathname on a string variable, like this:

int main(int ac, char **av)
{
  char*dir;

  if(ac > 2)
    {
      dir = malloc(sizeof(*dir) * 512);
      getcwd(dir, sizeof(*dir));
      printf("dat dir is:\n");
      printf("%s\n", dir);
    }
}

I got a blank printed, but when I do something like that

int main(int ac, char **av)
{
  char dir[512];

  if(ac > 2)
    {
      // dir = malloc(sizeof(dir) * 512);                                                                
      getcwd(dir, sizeof(dir));
      printf("dat dir is:\n");
      printf("%s\n", dir);
    }
}

it is printed properly, why? Isn't my first malloc suppose to make my variable like dir[512]

Answer 1

In

getcwd(dir, sizeof(*dir))

the sizeof is producing a 1 because *dir refers to a single char. This isn't what you want. If you replace this with 512, all ought to work fine.

The idiomatic coding would be something like this:

int main(int ac, char **av)
{
  const int buf_size = 512;
  char *dir;

  if(ac > 2)
    {
      dir = malloc(buf_size);
      getcwd(dir, buf_size);
      printf("dat dir is:\n");
      printf("%s\n", dir);
    }
}

Note multiplying by sizeof(*dir) is a no-op because it always returns 1. That is, malloc allocates in units of char. So I've omitted the no-op.

Answer 2

It has to do with arrays, pointers and the meaning of sizeof.

char dir[512];
sizeof(dir);

This is asking for the size of an array, which in this case is sizeof(char) * 512 -> 512.

char *dir;
sizeof(dir);

This is asking for the size of a pointer, which in this case is sizeof(char *) -> 4 or 8

char *dir;
sizeof(*dir);

This is an odd construct. I really don't feel comfortable with it. Because dir has not been initialized any dereferncing of dir is cause for concern. But I think the compiler is working out the correct thing here, which in this case is sizeof(char) -> 1.

When working if pointers, you need to carry around the size which you allocated.

char*dir;
if(ac > 2)
{
    dir = malloc(sizeof(char) * 512);
    getcwd(dir, sizeof(char) * 512);
    printf("dat dir is:\n");
    printf("%s\n", dir);
}

The right answer™ is a bit more complicated.

if (ac > 2)
{
    size_t size = 512;
    char *buf = malloc(size);
    char *dir = getcwd(buf, size);

    while (dir == NULL && errno == ERANGE)
    {
        size *= 2;
        buf = realloc(buf, size);
        dir = getcwd(buf, size);
    }

    printf("dat dir is:\n");
    printf("%s\n", dir);
    free(buf);
}

First thing, I drop sizeof(char). In C, sizeof(char) is defined to be 1, so it's redundant. Next, set dir as the return value of getcwd(3) and keep the data buffer separate. Be sure to check the return value, if getcwd(3) returns NULL and errno is ERANGE then buf was not big enough. In my example, I double the size of the buffer before trying getcwd(3) again. Finally, don't forget to free buf.

The thing I'm not doing here is checking the return value of malloc(3) or realloc(3). This is because I feel there is not point in this example. If I can't allocate the size of a path, then my program is going to crash no matter what I do.

Answer 3

Less things to go wrong if you put the size into a variable early on and then use that:

    {
      const sizeof_dir = sizeof(*dir)*512;
      dir = malloc(sizeof_dir);                                                                
      getcwd(dir, sizeof_dir);
      printf("dat dir is:\n");
      printf("%s\n", dir);
    }

What killed you was getcwd(dir, sizeof(*dir)), where the sizeof was still 1.

Answer 4

Whenever you dynamically allocates memory then always check the return value from calloc, malloca and realloc, because return value indicates that memory allocated successfully or not. Here the aforesaid first answer is good and to check the returned value is always helpful.