Thread: pointer-copying

Hello, i got problem with function which simply creates struct and using simple pointers copies for making a whole sturcture (Linked list). In first use of this function is nothing wrong, but in second use, values seems getting wrong. Better look at example and example of output.

Code:

row *create_row(int values[], char tab_name[]) {
    printf("2.1\n");
    if (find_tab(tab_name)->first_row != NULL) {
        print_row(find_tab(tab_name),find_tab(tab_name)->first_row);
    }
    tab *tab_temp = find_tab(tab_name);

    printf("2.2\n");
    row *actual = malloc(sizeof(row));
    item *first_item = calloc(tab_temp->columns_count,sizeof(item));

    printf("2.3\n");
    item *iter = malloc(sizeof(item));
    int i;
    if (tab_temp->first_row != NULL) print_row(tab_temp,tab_temp->first_row);

    printf("2.4\n");

    for (i = 0; i < tab_temp->columns_count; i++) {
        iter = (first_item + i*sizeof(item));
        if (i == ((tab_temp->columns_count) - 1)) {
            iter->next = NULL;
        }
        else {
            iter->next = first_item + ((i+1)*(sizeof(item)));
        }
        iter->value = values[i];
        printf("Value %d\n",iter->value);
    }

    if (tab_temp->first_row != NULL) print_row(tab_temp,tab_temp->first_row);

    printf("2.5\n");
    actual->first_item = first_item;
    actual->next = NULL;

    printf("2.6\n");
    if (tab_temp->first_row != NULL) print_row(tab_temp,tab_temp->first_row);
    if (tab_temp->first_row == NULL) {
        tab_temp->first_row = actual;
    }
    else {
        actual->next = tab_temp->first_row;
        tab_temp->first_row = actual;
    }
    if (tab_temp->first_row != NULL) print_row(tab_temp,tab_temp->first_row);

    printf("2.7\n");
    print_row(tab_temp,tab_temp->first_row);

    printf("2.8\n");
    return actual;
}

And Output:

Code:

2.1
2.2
2.3
2.4
Value 4
Value 5
Value 9
2.5
2.6
   He    Be    Ce 
    4     5     9 
2.7
   He    Be    Ce 
    4     5     9 
2.8
   He    Be    Ce 
    4     5     9 
2.1
   He    Be    Ce 
    4     5     9 
2.2
2.3
   He    Be    Ce 
    4     0 
2.4
Value 2
Value 7
Value 3
   He    Be    Ce 
    4     2     7     3 
2.5
2.6
   He    Be    Ce 
    4     2     7     3 
   He    Be    Ce 
    2     7     3 
2.7
   He    Be    Ce 
    2     7     3 
2.8
   He    Be    Ce 
    2     7     3 
3.

Look at this appearanc of magic number after 2.3 (at least for me magic). What comes wrong?

Code:

2.1
   He    Be    Ce 
    4     5     9 
2.2
2.3
   He    Be    Ce 
    4     0 
2.4

One more point, there appears warning when i debbuging:
warning: HEAP: Free Heap block 550fc0 modified at 551080 after it was freed
when allocing memory for row actual, but not only for this memory allocation

I feel that it is some basic fault but i cant find it.
Could you help me to find it? Or explain to me what i do wrong?
Thank you.

1. You malloc space for iter, then leak it away by assigning to it the value of a different pointer. If you want to copy into the newly malloc'ed space, you need to assign something to *iter instead.

2. When doing pointer arithmetic, you don't need to account for data-type-sizes, since that already happens.

Thank you for answer, i understand second point that i can accessing to items like to field.
But i have problem to understand first point well.

If i understood right:

Code:

*iter = first_item[i];

to pointer on iter i assigning pointer to item on i-th place.

But why the Heap problem when allocing memory for row actual

The message you got talked about something being modified after being freed -- there are no calls to free() in the code you posted so I have no idea what you were freeing. If you were trying to free iter, well, who knows where iter was pointing, so who knows where you were trying to free, etc.

Originally Posted by Holymanus

Code:

*iter = first_item[i];

This doesnt work because first_item[i] is item, not item *

Originally Posted by Holymanus

This doesnt work because first_item[i] is item, not item *

I think you've missed the point -- that's exactly why it does work. *iter is an item. first_item[i] is an item. Ergo everybody's happy.

What will go wrong is

Code:

iter->next = first_item + ((i+1)*(sizeof(item)));

There's nothing valid you can put here, assuming you are trying to make a chain of these things. You need to malloc a new item every time through the loop; and only once you've malloc'ed the new one do you know where to set the link to the old one.

And if i use calloc like field of structs ? and then to every each i assigning items and creating chain from new one to next one except situation of last one which will got there NULL pointer

Originally Posted by Holymanus

You are right, i have special funcions to free all this structs, but i didnt call them.

What im surprised about, is this whoknowswherefrom appeared pointer on first item of this first_row with 0 next item. How i notice in original post after 2.3 there has not appeared seqence 4 5 9 but 4 0 and nothing but only after allocating memory and this aritmetic in FOR.

So could you please explain me one more time what is wrong with struct iter (pointer on item)

When you do

Code:

iter->next = first_item + ((i+1)*(sizeof(item)));

you are stepping way the heck past any memory you own. (Since the sizeof(item) is implicitly done when doing pointer arithmetic, you are moving (i+1)*sizeof(item)*sizeof(item) bytes forward, wherever that takes you (nowhere good). It is not completely surprising that a later malloc call may finally give you that memory, which you then use.

So you have to decide whether you want a linked list, or an array. The calloc you have for first_item works fine for an array -- you get space for a whole row, it seems, and everybody's happy. But then you do not malloc each individual item, because, well, you already went and got space for it.

If you want a linked list, then the calloc is completely useless. You need to then do something like this (untested/uncompiled/typed directly into this text box):

Code:

Item *previous = NULL;
for (i = 0; i < tab_temp->columns_count; i++) {
    Item *current = malloc(sizeof(Item));
    current->value = values[i]; //or wherever you're getting the numbers from
    current->next = NULL; //either end of line or will get changed later
    if (previous) {
         previous->next = current; //hook current guy at end of chain
    } else {
        first_item = current; //if no previous then we must be first
    }
    previous = current; //set up next loop
}

to build your list.

Thank you very much, this is logical, and array was last chance which i didnt want use because all works on lists. I tryed it, and works well but seems i havent solved another problem but it is another fight so one more time thank you.

So i found one more problem when i add one more row it kills program after this for loop

Originally Posted by Holymanus

So i found one more problem when i add one more row it kills program after this for loop

You'll have to be way way way way way more explicit than that. What are you doing (i.e. the code) and what is going wrong (i.e. segmentation fault, or wrong answer, or what).