Jump to content
  • Advertisement
Sign in to follow this  
Stormtrooper

Duplication?

This topic is 3493 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have this(albeit messy) code that parses a string and fills a struct with the data. However, I get weird results.
int main(int argc, char **argv)
{
 
    Task *task = load_task("Check email,none,computer,0,1,1|");
    
    printf("%s\n", task_serialize(task));
    free(task);
    return 0;
}
char *task_serialize(Task *task)
{
    char *result = malloc(sizeof(task)); 
    sprintf( result, "%s,%s,%s,%i,%i,%i|",
            task->task,
            task->project,
            task->context,
            task->priority,
            task->done,
            task->nextAction);
                                                        
    return result;
}

Task *load_task(char *data)
{
    Task *result = malloc(sizeof(Task));
    
    // First get the task description
    char *i = strchr(data, ',');
    int in = i - data;
    char *task = malloc(in);
    strncpy(task, data, in - 1);
    
    result->task = task;
    free(task);
    
    // Remove the task desc.
    //strncpy(data, data + in, strlen(data));
    
    // Get the task project
    i = strchr(data, ',');
    in = in + i - data;
    char *project = malloc(in);
    strncpy(project, data, in - 1);
    
    result->project = project;
    free(project);
    
    // Remove the task desc.
    //strncpy(data, data + in, strlen(data));
    
    // Get the context
    i = strchr(data, ',');
    in = in + i - data;
    char *context = malloc(in);
    strncpy(context, data, in - 1);
    
    result->context = context;
    free(context);
    
    // Remove the task desc.
    //strncpy(data, data + in, strlen(data));
    
    // Get the priority
    i = strchr(data, ',');
    in = in + i - data;
    char *priority = malloc(in);
    strncpy(priority, data, in - 1);
    
    result->priority = atoi(priority);
    free(priority); 
    
    // Remove the task desc.
    //strncpy(data, data + in, strlen(data));
    
    // Get if the task is done
    i = strchr(data, ',');
    in = in + i - data;
    char *done = malloc(in);
    strncpy(done, data, in - 1);
    
    result->done = atoi(done);
    free(done);
    
    // Remove the task desc.
    //strncpy(data, data + in, strlen(data));
    
    // Get if the task is next action
    i = strchr(data, ',');
    in = in + i - data;
    char *nextaction = malloc(in);
    strncpy(nextaction, data, in - 1);
    
    result->nextAction = atoi(nextaction);
    free(nextaction);
    
    return result;
}
I get this weird output: Check email,none,computer,0,1,1|,,computer,0,1,1|,,0,0,0| It should be what I inputted: Check email,none,computer,0,1,1|

Share this post


Link to post
Share on other sites
Advertisement

char *task_serialize(Task *task)
{
char *result = malloc(sizeof(task));


It seems that you allocate only 4 (8) bytes for result (sizeof pointer). If the Task struct itself contains char pointers I would guess it is impossible to know how much memory you'll need unless the task object itself knows it (the total length of string fields + separators + null terminator).


char *task = malloc(in);
strncpy(task, data, in - 1);

result->task = task;
free(task);


You allocate memory, put some data in it, assign the pointer to a struct member and immediately deallocate the memory, effectively rendering the pointer within the Task struct invalid.

Share this post


Link to post
Share on other sites
So I tried this to get a better estimate for the size:


int size = strlen(task);
size += strlen(project);
size += strlen(context);
// one for priority
size += 1;
// one for done bool
size += 1;
// one for nextAction bool
size += 1;

Task *result = malloc(size);

result->task = task;
result->project = project;
result->context = context;
result->priority = atoi(priority);
result->done = atoi(done);
result->nextAction = atoi(nextaction);


And got this output:
Check emai,Check email,none,comp,Check email,none,computer,0,1,1|,0,0,0|

should be
Check email,none,computer,0,1,1|

Share this post


Link to post
Share on other sites
Are you also taking into account that strings need one extra character worth of space for the null-terminator?

Could you post a full compilable example (the two functions, the struct definition and main - together with includes)?

Share this post


Link to post
Share on other sites
To allocate an instance of task struct, you could use something like this:

Task *make_task(const char *task,const char *project,const char *context,priority p,bool done,bool nextAction)
{
Task *t = malloc(sizeof(Task));
t->task = strdup(task);
t->project = strdup(project);
t->context = strdup(context);
t->priority = p;
t->done = done;
t->nextAction = nextAction;
}

void free_task(Task *t)
{
if(t)
{
free(t->task);
free(t->project);
free(t->context);
free(t);
}
}


The free_task() assumes that the task is always allocated with make_task(). Note that we have 4 calls to free(). This matches with the number of allocations, 1 to malloc() and 3 to strdup() (which is documented as returning a pointer that can be deallocated with a call to free).

Share this post


Link to post
Share on other sites
Well, one more time...


char *task_serialize(Task *task)
{
char *result = malloc(sizeof(task));
...


What do you think sizeof(task) is and is it enough to hold all the data in the Task object? (Note, sizeof(*task) wouldn't help you either.)

Another very similar thing


char *result = malloc(sizeof(getCwd()));
sprintf(result, "%s/", getCwd());


Do you know what is the difference between sizeof and strlen? sizeof is something that evaluates size of stuff at compile-time (a char* in this case), it is impossible that it would give you the length of a string that is known only at run-time.

It might still be better to provide a minimal example, just something that can be easily compiled without additional dependencies.

Share this post


Link to post
Share on other sites
Okay, I understand what you are saying. Shouldn't this work?

char *task_serialize(Task *task)
{
int size = strlen(task->task);
size += strlen(task->project);
size += strlen(task->context);
// one for priority
size += 1;
// one for done bool
size += 1;
// one for nextAction bool and null termination
size += 2;

char *result = malloc(size);

//memcpy((void *)result, (void *)task, sizeof(task));
sprintf( result, "%s,%s,%s,%i,%i,%i|",
task->task,
task->project,
task->context,
task->priority,
task->done,
task->nextAction);

return result;
}

Share this post


Link to post
Share on other sites
Hmm, I misread what you were trying to do. I think writing directly to a file would be easier, because that is what the calling code inevitably appears to be doing:

void task_serialize(Task *task, FILE *dest);

At the moment it appears that your code doesn't take into account the commas and the pipe character that is in the format string.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!