Basic C question, concerning memory allocation and value assignment

Question

Hi there,

I have recently started working on my master thesis in C that I haven't used in quite a long time. Being used to Java, I'm now facing all kinds of problems all the time. I hope someone can help me with the following one, since I've been struggling with it for the past two days.

So I have a really basic model of a database: tables, tuples, attributes and I'm trying to load some data into this structure. Following are the definitions:

typedef struct attribute
{
    int type;
    char * name;
    void * value;
} attribute;

typedef struct tuple
{
    int tuple_id;
    int attribute_count;
    attribute * attributes;
} tuple;

typedef struct table
{
    char * name;
    int row_count;
    tuple * tuples;
} table;

Data is coming from a file with inserts (generated for the Wisconsin benchmark), which I'm parsing. I have only integer or string values. A sample row would look like:

insert into table values (9205, 541, 1, 1, 5, 5, 5, 5, 0, 1, 9205, 10, 11, 'HHHHHHH', 'HHHHHHH', 'HHHHHHH');

I've "managed" to load and parse the data and also to assign it. However, the assignment bit is buggy, since all values point to the same memory location, i.e. all rows look identical after I've loaded the data. Here is what I do:

char value[10]; // assuming no value is longer than 10 chars
int i, j, k;

table * data = (table*) malloc(sizeof(data));
data->name = "table";
data->row_count = number_of_lines;
data->tuples = (tuple*) malloc(number_of_lines*sizeof(tuple));

tuple* current_tuple;

for(i=0; i<number_of_lines; i++)
{
    current_tuple = &data->tuples[i];
    current_tuple->tuple_id = i;
    current_tuple->attribute_count = 16; // static in our system
    current_tuple->attributes = (attribute*) malloc(16*sizeof(attribute));

    for(k = 0; k < 16; k++)
    {
        current_tuple->attributes[k].name = attribute_names[k];

        // for int values:
        current_tuple->attributes[k].type = DB_ATT_TYPE_INT;
        // write data into value-field
        int v = atoi(value);
        current_tuple->attributes[k].value = &v;

        // for string values:
        current_tuple->attributes[k].type = DB_ATT_TYPE_STRING;
        current_tuple->attributes[k].value = value;
    }

    // ...
}

While I am perfectly aware, why this is not working, I can't figure out how to get it working. I've tried following things, none of which worked:

 memcpy(current_tuple->attributes[k].value, &v, sizeof(int));

This results in a bad access error. Same for the following code (since I'm not quite sure which one would be the correct usage):

 memcpy(current_tuple->attributes[k].value, &v, 1);

Not even sure if memcpy is what I need here...

Also I've tried allocating memory, by doing something like:

 current_tuple->attributes[k].value = (int *) malloc(sizeof(int));

only to get "malloc: * error for object 0x100108e98: incorrect checksum for freed object - object was probably modified after being freed." As far as I understand this error, memory has already been allocated for this object, but I don't see where this happened. Doesn't the malloc(sizeof(attribute)) only allocate the memory needed to store an integer and two pointers (i.e. not the memory those pointers point to)?

Any help would be greatly appreciated!

Regards, Vassil

Answer 1

So for strings you're saving a pointer to the string in the value field of an attribute, but for integers you want to put the integer itself in the value field? Then you're trying too hard. Just use:

current_tuple->attributes[k].value = (void *)v;

If you want to save a pointer to the integer, you're going to need to allocate space for it, since storing a pointer to a locally-scoped variable is going to end in tears. Something like this:

int *v = malloc(sizeof(int));
*v = atoi(value);
current_tuple->attributes[k].value = v;

Likewise for the strings, you're always storing the same local variable pointer value into your data structures. You should probably be doing some copying or memory allocation to keep from constantly overwriting your data.

Answer 2

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

#define STR_TYPE 1
#define INT_TYPE 2

typedef struct tagAttribute
{
    int type;
    char *name; 
    // anonymous union here
    union {
        char *value;
        int  ivalue;
    };
} attribute, *PATTRIBUTE;


typedef struct tagTuple
{
    int tuple_id;
    int attribute_count;
    attribute *attributes;
} tuple, *PTUPLE;


typedef struct tagTable
{
    char *name;
    int row_count;
    tuple *tuples;
} table, *PTABLE;

    // allocator for table 
PTABLE allocTable(char* name, int row_count) {
    PTABLE mytable = calloc(sizeof(table),1);
    mytable->row_count = row_count;
    mytable->name = strdup(name);
    mytable->tuples = (PTUPLE)calloc(row_count,sizeof(tuple));
    for(int i=0; i< row_count; i++) {
        mytable->tuples[i].tuple_id= i; // ?
    }
    return(mytable);
}

    // allocator for attributes
void allocAttributes( PTUPLE mytuple,  int attr_count) {
    mytuple->attributes =   (PATTRIBUTE) calloc(attr_count, sizeof(attribute));
    mytuple->attribute_count = attr_count;
}

void setAttributeStr(PATTRIBUTE pa, char *name, char *value) {
    pa->type = STR_TYPE;
    pa->name = strdup(name);
    pa->value = strdup(value);
}

void setAttributeInt(PATTRIBUTE pa, char *name, int value) {
    pa->type = INT_TYPE;
    pa->name = strdup(name);
    pa->ivalue = value;

}

int main (int argc, const char * argv[]) {
    // insert code here...
    printf("Test!\n");

        // allocate a table with two tuples
    PTABLE mytable = allocTable("my_table", 2);
        // allocate two attributes in the first tuple

    PTUPLE t0 = & mytable->tuples[0];
    PTUPLE t1 = & mytable->tuples[1];

    allocAttributes( t0, 2);
    allocAttributes( t1, 2);

    setAttributeStr(  &t0->attributes[0], "my_strng_field", "value0");
    setAttributeInt( &t0->attributes[1], "my_int_field", 0xBEEF);
        // now assign 
    setAttributeStr(  &t1->attributes[0], "my_second_strng_field", "value0");
    setAttributeInt( &t1->attributes[1], "my__second_int_field", 0xDEAD);


    return 0;
}

• you need to be careful with your allocations
• or use strdup() for strings so they take care of this for you

Works on my MAC.

Thanks for a trip down memory lane (15 years or so)

Answer 3

So, you have several things going on here. First, don't cast the return value of malloc in the C programming language. Next, this is going to cause troubles:

int v = atoi(value);
current_tuple->attributes[k].value = &v;

You are having value point to memory allocated on the stack. That's bad if you access it after that current "v" has gone out of scope.

Also, you are not using any branching condition to determine if the value should be string or int. Therefore, you will end up with memory leaks, assuming you assigned things properly as mentioned before. I suspect this is partly because it is just an example.

Check the return value of malloc. You can create a wrapper function to do this for you. Also, you may want to do some better logging.

Essentially, you need to become more familiar with how pointers work in C and the difference between allocating on the heap and allocating on the stack. Stack or automatic variables go out of scope. When you malloc it stays around forever until you get rid of it. Don't ever set a pointer equal to the memory location of something allocated on the stack unless you really mean to do that (again, your example with "v", that memory address will be invalid as soon as that particular iteration of the loop is finished, the worst case here is that it works when you test it and you don't notice the error.

Additionally, "//" is not ANSI-C89 style comment. Use "/" and "/"

I made a couple changes; I can't guarantee it will work now as I haven't tested it obviously. However, I recommend reading the C Programming Language

char value[10]; /* assuming no value is longer than 10 chars */
int i, j, k;

table * data = malloc(sizeof(table));
     if(!data)
         exit(1); /* error */

data->name = "table";
data->row_count = number_of_lines;
data->tuples = malloc(number_of_lines*sizeof(tuple));
     if(!data->tuples)
         exit(1); /* error */

tuple* current_tuple;

for(i=0; i<number_of_lines; i++)
{
    current_tuple = &data->tuples[i];
    current_tuple->tuple_id = i;
    current_tuple->attribute_count = 16; /* static in our system */
    current_tuple->attributes = malloc(16*sizeof(attribute));

    for(k = 0; k < 16; k++)
    {
        current_tuple->attributes[k].name = attribute_names[k];

                        if(k % 2)
                        {
               /* for int values:*/
               current_tuple->attributes[k].type = DB_ATT_TYPE_INT;
               /* write data into value-field */
                               current_tuple->attributes[k].value = malloc(sizeof(int));
                               if(!current_tuple->attributes[k].value)
                               {
                                    exit(1); /* error */
                               }    
               *current_tuple->attributes[k].value = atoi(value);

                        }
                        else
                        {
               /* for string values: */
               current_tuple->attributes[k].type = DB_ATT_TYPE_STRING;
                               current_tuple->attributes[k].value = malloc(strlen(value) +1);
                               if(!current_tuple->attributes[k].value)
                               {
                                    exit(1); /* error */
                               }
               strcpy(current_tuple->attributes[k].value, value);
                        }
    }


}