0
typedef struct  
{
    char path[MAX_FILENAME*MAX_FOLDERS];
    char filename[MAX_FILENAME];
    time_t date;
    off_t size;
} FILES;

This code works

FILES *fls = (FILES*)malloc(sizeof(FILES));
    strcpy(fls[0].filename, "asd");
    printf("%s\n", fls[0].filename);
    fls = (FILES*)realloc(fls, 2);
    strcpy(fls[1].filename, "asdfgh");
    printf("%s\n", fls[0].filename);
    printf("%s\n", fls[1].filename);

But here:

void allocateFileTree(FILES *tree,int i)
{
    if(i==0)
      tree = (FILES*)malloc(sizeof(FILES));
    else
      tree = (FILES*)realloc(tree, sizeof(FILES)*i);      

}

in a loop

allocateFileTree(tree, i);
struct stat buff;
stat(entry -> d_name, &buff);

strcpy(tree[i].path, "whatever");//it gives segfault
i++;//this is never executed so realloc isn't the problem (yet)

Why and how can I solve this? What is so different that it crashes?

Mihai Bratulescu
  • 1,915
  • 3
  • 27
  • 43
  • What is the declaration of `tree` in your "in a loop" scope? It is apparently a pointer or array. You seem to intend to modify it in `allocateFileTrees()`, but that function modifies only the local pointer value and does not modify the passed-in pointer. That would require a double pointer. – Fred Larson Mar 17 '14 at 19:37
  • I've declared tree global and replaced allocateFileTree() with it's code but it still gives segfault – Mihai Bratulescu Mar 17 '14 at 20:38

3 Answers3

4

The code you say works really doesn't. One major problem is this line here

fls = (FILES*)realloc(fls, 2);

This reallocate the pointer to be two bytes. There's also a problem with this if the realloc call fails, as then you overwrite the only pointer you have with NULL, and therefore loose the original pointer and have a memory leak (besides the obvious problem of dereferencing a NULL pointer).

Your exact cause of the crash is because you don't allocate memory for the path member, so you're using an uninitialized pointer.

Both of the above leads to undefined behavior, which is a common cause of crashes.

And finally, in C you should not cast the return of malloc (and family).

Community
  • 1
  • 1
Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • my mistake path is now [MAX] but the first code works, i think the first comment describes my problem – Mihai Bratulescu Mar 17 '14 at 20:11
  • @MihaiBratulescu The first piece of code just *seem* to work, in reality you are overwriting memory you haven't allocated. The size given to `realloc` is the number of *bytes* you want to allocate. – Some programmer dude Mar 17 '14 at 20:20
  • in the second piece of code realloc is correct but I still get segfault! How can I solve this? – Mihai Bratulescu Mar 17 '14 at 20:43
  • @MihaiBratulescu The expression `sizeof(tree)` returns the size of *the pointer*, not the size of the structure. – Some programmer dude Mar 17 '14 at 20:48
  • changed it to FILES but I have the same problem, anyway it never gets to i++ it crashes at the first appending – Mihai Bratulescu Mar 17 '14 at 20:51
  • 1
    @MihaiBratulescu And you *do* initialize `i` to zero first? – Some programmer dude Mar 17 '14 at 20:53
  • yes but I had incremented it before and forgot, thanks for the suggestion now it's working – Mihai Bratulescu Mar 17 '14 at 20:58
  • @MihaiBratulescu There's actually another problem with your code, and I'm surprised it works, because you modify the argument `tree`, but in C all arguments are passed by value (i.e. copied) so you're only modifying the *local copy* of that pointer. The function calling `allocateFileTree` will not see the changes in the `allocateFileTree` function as those are only local in that function. – Some programmer dude Mar 18 '14 at 08:12
1

While you allocate space for an array of FILES, you do not allocate storage for path in the code shown.

In the code

strcpy(tree[i].path, "whatever")

The value of tree[i].path is undefined. It might happen to point to space you can write to, or not.

Eric J.
  • 147,927
  • 63
  • 340
  • 553
0

This statement:

(FILES*)realloc(tree, sizeof(tree)*i);

allocates enough space for i pointers since tree is a FILES*. I think you want:

(FILES*)realloc(tree, sizeof(*tree)*i);

Your other problem is that you never actually update your tree pointer. The allocateFileTree() function only every updates it's local copy of the pointer to the new allocation.

You may want to try something like

FILES* allocateFileTree(FILES *tree,int i)
{
    if(i==0)
      tree = (FILES*)malloc(sizeof(FILES));
    else
      tree = (FILES*)realloc(tree, sizeof(FILES)*i);      

    return tree;
}

and call it like so:

tree = allocateFileTree(tree, i);
Michael Burr
  • 333,147
  • 50
  • 533
  • 760