memory error caused by self created tostring function in C

Question

This is a function I created to print the content of a structure as a single string, basically it acts like a tostring function.

typedef struct {
    
    char* firstname;

    char* lastname;

    char* address;
    
} info;

char* strucstring(struct* info)
{ 
    char * string = (char*)malloc(sizeof(info->firstname));
    sprintf(string, "%s",info->firstname);
    
    return string;
}

In my main when I call it like this:

char *string  = strucstring(info);
printf("%s",string);
free(string);

return 0;

There is no memory leak but there are some memory errors. You can see that there are no memory leaks but 24 errors in 8 contexts.

How do I remove these errors??

`sizeof(info)` is wrong. Also your code is wrong, it doesn't even compile; presumably you mean `struct something *info`. In any case you probably mean `sizeof info->name`, which *could fix it* or not depending on the declaration of the string. Finally, since you seem to be on a Unix system, then do use `char *string = strdup(info->name)` instead of malloc/sprintf — Antti Haapala -- Слава Україні, Feb 02 '21 at 13:55
What is `info`? its definition is not included. No matter what it is, the expression `sizeof(info)` will provide the size of a pointer, probably not what you intended. Hint, change prototype to char* strucstring(struct* info, size_t size)`, and pass the _sizeof_ `info` as an argument. — ryyker, Feb 02 '21 at 13:57
I made an edit. There are multiple things in info, that is why I used malloc(info).. — Rocket, Feb 02 '21 at 13:58
How do malloc a string for all the struct contents like firstname lastname and address?? Do I realloc? — Rocket, Feb 02 '21 at 13:59
Normally you need to allocate memory for each pointer member in a struct. Since we don't how the struct is declared, we can't really answer the question. But `sprintf(string, "%s %s %s",info->firstname, info>address, info->lastname);` suggests that you have a fundamental misunderstanding about how pointers and arrays work. I'd step back a few chapters in the C programming book and read up on arrays & pointers before proceeding to malloc. — Lundin, Feb 02 '21 at 14:04
` info>address` is not a `%s` pointer but either 1 or 0. Also I suggest to use snprintf instead. — Devolus, Feb 02 '21 at 14:05
@Rocket how do you initialize `info` before the call to `strucstring(info)`? There could also be a problem _there_. — Jabberwocky, Feb 02 '21 at 14:25
_"How do malloc a string for all the struct contents like firstname lastname and address?? Do I realloc?"_ Look at the answer, it likely explains the correct approach, depending on how your created your struct. Edit your post to show that. — ryyker, Feb 02 '21 at 14:44
@Jabberwocky Then you of course modify the code according to that — klutt, Feb 02 '21 at 14:52

Jabberwocky · Answer 1 · 2021-02-02T14:46:47.787

Your memory allocation here is wrong. sizeof(info) is the size of the structure but not the size of the strings in the structure. Also (struct* info) is wrong, it should be (struct info* info)

char* strucstring(struct* info)
{ 
    char * string = (char*)malloc(sizeof(info));  // <<< this is wrong
    sprintf(string, "%s %s %s",info->firstname, info->address, info->lastname);    
    return string;
}

You want this:

char* strucstring(struct info* info)
{ 
    int sizeneeded = 1 + snprintf(NULL, 0, "%s %s %s",info->firstname, info->address, info->lastname);
    // the +1 is for the NUL string terminator

    char *string = malloc(sizeneeded);    // the (char*) cast is not needed
    snprintf(string, sizeneeded, "%s %s %s",info->firstname, info->address, info->lastname);    
    return string;
}

We call snprintf with a 0 sized buffer. This will return the number of bytes needed for the output string. Then we allocate the number of bytes needed and call snprintf a second time this time with the newly allocated buffer.

Disclaimers:

no error checks are done here for brevity.
there might be more problems elsewhere in the parts of your code you didn't show.

Your allocation approach is the right way to go here. – ryyker Feb 02 '21 at 14:42 — ryyker, Feb 02 '21 at 14:42

ryyker · Answer 2 · 2021-02-02T15:51:22.827

The prototype: char* strucstring(struct* info); throws the following errors:

6, 19 error: declaration of anonymous struct must be a definition
6, 19 error: expected parameter declarator

Change it to ((based on modification of your edit to the typedefed struct, see below):

char* strucstring(info_s *info);

(There are other errors as well. Compile your code with warnings turned on to see all of them.)

~~Assuming a minimal definition of your struct:~~

typedef struct { char firstname[80]; char lastname[80]; char address[80]; }info_s;

Given your struct definition (with minor modifications from your post's edit.)

typedef struct {  
    char* firstname;
    char* lastname;
    char* address;    
} info_s;

Each of the members needs to be populated before passing as an argument. This can be done in several ways, simple assignment statements are used below

An example of your code adapted with corrections, including proper way of allocating memory for string: (Credit to @Jabberwocky, +1)

typedef struct {
    char *firstname;
    char *lastname;
    char *address;
}info_s;

char* strucstring(info_s *info);

int main(void)
{   
    //populate each struct member with content:
    info_s info = {"Roger", "Bains", "1234, Noble Rd, Christchurch"};
    
    char *string  = strucstring(&info);
    printf("%s",string);
    free(string);

    return 0;
}

char* strucstring(info_s *info)
{ 
    int sizeneeded = 1 + snprintf(NULL, 0, "%s %s %s",info->firstname, info->lastname, info->address);
    char * string = malloc(sizeneeded);
    sprintf(string, "%s %s %s",info->firstname, info->lastname, info->address );
    
    return string;
}

Note also the cast has been removed from malloc() for these reasons.

memory error caused by self created tostring function in C

2 Answers2