Freeing allocated memory conditionally within a macro in C

0
votes

I have a function that reads a csv file and then updates a struct with it's parameters. I wanted to be able to cycle through a structs elements, so I turned to macros. The output of parsing the csv files is a 2 dimensional array of strings for rows and columns of the file. To convert the strings to their respective data type (currently the structs only have int and char*) I used a conversion macro within the macro used to cycle through the struct.

CVT_INT atoi(str)
CVT_STR str

However, when it comes to freeing the memory allocated by parsing the csv file, it gets tricky if in the file the strings are not grouped together at the start or end.

csv[row][col]
string|string|int|string|int
string|string|int|string|int
...

for(int row = 0; row < number_of_rows; row++)
    for(int col = 2; col < number_of_cols; col++)
        free(csv[row][col]) // frees string when row[i][3]

I could just make sure all the strings are at the beginning of the structure, but I want it to be dynamic and I don't want to have to think about making sure the data types are grouped.

I could free the allocated strings that were converted by CVT_INT, but freeing the strings used by CVT_STR would result in the struct's strings being freed. I could think of one workaround: 1. Allocate new space 2. Copy in the old string 3. Free the old string.

CVT_STR strcpy((char*)malloc(sizeof(char)*(1+strlen(str))), str)

However, on implementing the above, it leads to a crash whenever it is called, and I don't understand why. Could anyone offer me an explanation and a way to solve it/a different route that does the same job? I'm aware it's not very efficient, so suggestions on improving that aspect are also welcome.

Another possibility, I could free only int. However, I couldn't work out how to do this in the macro, as it has to return an int.

Below is an example of me calling the cycle-through-struct macro, containing the conversion macro.

#define STRUCT(type, name, converter) \
        obj->struct.name = converter(csv[row][col++]);
STRUCT_FIELDS
#undef PLAYER

Thanks for any help

UPDATE:

Replacing

CVT_STR strcpy((char*)malloc(sizeof(char)*(1+strlen(str))), str)

with

CVT_STR strdup(str)

worked, but I don't understand why. Perhaps someone could enlighten me?

2
cmemory-managementmacros
Why are you using macros for this, and not inline functions [at least for the final step, you can still use macro to do the expansion of converter, but pass a function instead of a macro as the "converter". That way, you can debug the code (and it most likely isn't any less efficient, as the compiler will inline the function!) – Mats Petersson
Does your system lack strdup()? – jxh
@user315052: Since the comment on the existing answer says that the strcpy() doesn't work, I suspect that a function that does exactly that same thing [or do you think that strdup does something different from strcpy(malloc(strlen(str)+1), str) - sure, it does some checking if malloc is null, and such, but otherwise it's the same. – Mats Petersson
Thank you user315052, I have not seen that function before and it has solved the problem! From reading the man pages it seems to do exactly the same as I hard coded with strcpy, but for some reason it works where my code didn't. Maybe the macro didn't like the typecasting of malloc. Inline functions - I am not familiar with them, but I was recommended to use macros to cycle through the struct. I am guessing that using the macro to to do this therefore dictates I can't use inline functions? I will look into this however, as the lack of debugging functionality is already frustrating. – Richard
I am guessing you did not #include <stdlib.h>, so your cast hid the missing prototype. With the missing prototype, and if int is smaller than a pointer, you malloc()s return value will get truncated, resulting in an invalid pointer. – jxh

2 Answers

2
votes

You need to add one to strlen which does not included the trailing NUL byte.

CVT_STR strcpy((char*)malloc((sizeof(char) * strlen(str)) + 1), str)

or simply

CVT_STR strcpy((char*)malloc(strlen(str) + 1), str)
1
votes

So, we have a macro that converts things:

#define STRUCT(type, name, converter) \
    obj->type.name = converter(csv[row][col++]);    // Guessing `struct` meant `type`?

And it gets called with something like this:

STRUCT(foo, bar, CVT_STR)

Now, if CVT_STR is a macro, the macro gets expanded directly into the STRUCT macro expansion, like this:

obj->foo.bar = strdup(csv[row][col++]);

Now, if you wanted to step through this, you'd have a hard time, since it just does the macro as one step.

If instead, we write a function:

inline char *cvt_str(const char *str)
{
    char *tmp = strdup(str); 
    return tmp;
}

and use this function like this:

STRUCT(foo, bar, cvt_str)

now we can use the debugger to set a breakpoint in cvt_str, and step through it to see where it's going wrong.