Unable to malloc string in struct

Question

My struct looks like this:

struct tree{
    char *name;
    int num_subdirs;
    struct tree **subdirs;
}

I am receiving a buffer that contains this entire tree serialized in a buffer. I am trying to deserialize it in this function:

struct tree *t;
//buffer is filled, received from somewhere else.
int res = deserialize(t, buf); //call function deserialize

//deserialize function
            //buf = {../,2}{sd,0}{|}{sr,1}{sk,0}{|}{|}
   │406     int dfsDeserialize(struct tree *dt, void *buf, int *q){                                                                                                                       │
   │407         char name[MAXPATHLEN];                                                                                                                                                           │
   │408         char delim[3];                                                                                                                                                                   │
   │409         int len, numsubs, i;                                                                                                                                                             │
                                                                                                                                                        │
   │411         sscanf(buf+(*q),"%3s",delim);                                                                                                                                                    │
   │412         if(!strcmp(delim,"{|}")){                                                                                                                                                        │
   │413             (*q)+=3;                                                                                                                                                                     │
   │414             return 1;                                                                                                                                                                    │
   │415         }                                                                                                                                                                                │
   │416         sscanf((buf + (*q)), "{%[^,],%d}%n", name, &numsubs, &len);                                                                                                                      │                                                                                                                                          │
  >│419         int slen = strlen(name);                                                                                                                                                         │
   │420         dt->name = calloc(slen + 1, 1);                                                                                                                                                  │
   │421         dt->subdirs = malloc(numsubs*sizeof(struct tree *));                                                                                                                      │
   │422         strcpy(dt->name, name);                                                                                                                                                          │
   │423         dt->num_subdirs = numsubs;                                                                                                                                                       │
   │424         (*q)+=len;                                                                                                                                                                       │
   │425         for(i = 0; i< numsubs; i++){                                                                                                                                                     │
   │426             dt->subdirs[i] = malloc(sizeof(struct tree));                                                                                                                         │
   │427             dfsDeserialize(dt->subdirs[i], buf, q);                                                                                                                                      │
   │428         }                                                                                                                                                                                │
   │429         return 0;                                                                                                                                                                        │
   │430     }   

                                                                                                                                                                             │

I have tried several different ways of allocating memory for string but it fails every single time! I don't know why is t->name always 0x0. Please help.

Since you've skipped the code that parses the buffer and the code that we can see looks OK, we have to assume that the code you have omitted caused the trouble — or you have some memory problems elsewhere that finally trigger a problem here. Are you on a platform where you can use valgrind? If so, use it. If not, life is harder. — Jonathan Leffler
– Jonathan Leffler, Commented Feb 6, 2016 at 8:20
note that t is like a local variable and what it points to after you leave the function will be lost. if you want to permanently change what t points to you need to pass the address of t — AndersK
– AndersK, Commented Feb 6, 2016 at 8:25
You have a buffer overflow. The size in your format string is %3s but the array is only 3 bytes long. Unfortunately, the scanf() functions take the size excluding the null terminator, so you have a one-byte overwrite potential, which can be disastrous. Increase the size of delim to at least 4, or change the format to specify 2 instead of 3. Whether that's the cause of all your trouble is open to debate, but buffer overflows are never good news. — Jonathan Leffler
– Jonathan Leffler, Commented Feb 6, 2016 at 8:59

AndersK · Accepted Answer · 2016-02-06 08:32:36Z

2

I think the culprit is this

t = malloc(sizeof(sizeof tree));

You probably meant

t = malloc(sizeof(struct tree));

you can also use the more handier strdup to copy a string on the heap

t->name = strdup(name);

edited Feb 6, 2016 at 8:32

answered Feb 6, 2016 at 8:30

AndersK

36.1k6 gold badges63 silver badges86 bronze badges

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

4 Comments

Edgar Rokjān Over a year ago

I think the same. It looks like a bug.

AndersK Over a year ago

@EdgarRokyan yes, a typo

Tom Karzes Over a year ago

That's definitely a bug, but not the only one.

Neo Over a year ago

@TomKarzes I've updated representational code to the actual code I am using.

M.M · Accepted Answer · 2016-02-06 08:53:11Z

C uses "pass by value". When you pass an argument to a function, the function receives a copy of the argument. So when you have:

struct tree *t;
int res = deserialize(t, buf); //call function deserialize

nothing that the function does can affect t. The function gets a copy of the current value of t. (Since t is uninitialized, this may cause undefined behaviour).

Instead you need to tell the function where t is found in memory:

int res = deserialize(&t, buf);

and the function might look like:

int deserialize(struct tree **p_t, void *buf)
{
    struct tree *t = malloc(sizeof *t);
    // ... set up t as before

    // tell the caller about what we did
    *p_t = t;
}

An alternative approach would be to leave deserialize as it is but remove the malloc entirely; and require that the caller allocates space before calling the function. This is more flexible as then the caller can choose to use automatic allocation or dynamic allocation.

Okay, but what about the children node in consequtive calls? I would need to malloc them, right?
NOTE: this answer was written before the latest edit, in which memory is never allocated at any stage
Sorry about that. I thought for clarity it would be better to use the actual code.
@Neo you mean subdirs? Hard to say since the existing code is incomplete.

Collectives™ on Stack Overflow

Unable to malloc string in struct

2 Answers 2

4 Comments

8 Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

8 Comments

Related