Copying component of struct deletes another component of the same struct

Question

I am currently learning C and have to program a "dynamic array".

In a header file provided to us, the struct DynArray is declared as

struct DynamicArray
{
    unsigned int size;
    unsigned int capacity;

    int *data;
};

I have already implemented most of the functions in the dyn_array program, which was provided with empty functions.
My difficutly lies with the function dn_append(DynamicArray *a, int elem). The only description I have been given is

// =====================================================================================
//         Name:  dn_append
//  Description:  Append an element.
//
//   Parameters:  a - the array
//                elem - the new value
//      Returns:  non-zero, if the array was successfully extended
// =====================================================================================

We have a makefile to compile this and a few test cases. In one of the test programs, a new DynArray is initialized and then a few values are appended:

int
main ( int argc, char *argv[] )
{
    DynamicArray *a1;
    int                     i;

    a1 = dn_new ( 5 );

    dn_append ( a1,  5 );
    dn_append ( a1,  7 );
    dn_append ( a1,  8 );
    dn_append ( a1, 11 );
    dn_append ( a1, 13 );
    dn_append ( a1, 15 );

    dn_set ( a1, 2, 9 );

    for ( i = 0; i < dn_size ( a1 ); i += 1 )
    {
        printf ( "%d\n", dn_get ( a1, i ) );
    }

    dn_destory ( a1 );

    return 0;
}

It aborts with a segmentation fault.
My (faulty) implementation is as follows. The outer else-case is completely messed up, since the debugging drove me crazy. (Note that I explain the problematic line after the code sample.)

    int
dn_append ( DynamicArray *a, int elem )
{
    printf("\n\nAppend:\n");
    if (a->size >= a->capacity) {
        printf("Array too small");
        int *dataPtr = realloc(a->data, 2*a->capacity);

        if (dataPtr != NULL) {
            a->capacity *= 2;
            a->data = dataPtr;
            a->data[a->size] = elem;
            a->size++;
        }
        else {
                return 0;
        }
    }
    else {
        int *dataPtr;
        dataPtr = a->data;

        printf("Size: %d, Capacity: %d\n", a->size, a->capacity);
        int sizeN = a->size;
        printf("Size: %d, Capacity: %d\n", a->size, a->capacity);

        //int offset = sizeN;
        int *temp;
        temp = dataPtr;// + offset;
        //dataPtr[offset] = elem;
        //printf("Data at %d is %d, should be %d\n", offset, *(a->data), elem);

        a->size++;
    }

    return 1;
}

The problematic line is in the outer else-case, in the middle:

    printf("Size: %d, Capacity: %d\n", a->size, a->capacity);
    int sizeN = a->size;
    printf("Size: %d, Capacity: %d\n", a->size, a->capacity);

When I run the program, these lines print out

Size: 0, Capacity: 5
Size: 0, Capacity: 0

I don't even touch the capacity-component of the struct, but it sets it to 0, which completely f***s up the following program.

After commenting the line int sizeN = a->size;, the capacity is left right as it should.
I need to read the size, one way or another.

So, why the hell does it change that component?

Some additional infos:

DynamicArray*
dn_new ( unsigned int capacity )
{
    if (capacity > 0) {
        int *dataPtr = malloc(capacity*sizeof(int));

        if (dataPtr != NULL) {
            struct DynamicArray array = { 0, capacity, dataPtr };
            return &array;
        }
        else {
            return NULL;
        }
    }
    else {
        return NULL;
    }
}

This is the type of well-written homework question that I would spend time reading and possibly answering. +1. — user12205, May 02 '15 at 13:18
Please show your allocator function, and try running your code in `valgrind` and/or a debugger. — Mat, May 02 '15 at 13:21
Probably has nothing to do with the error that you encounter, but you should print `unsigned` values with `%u`. — Jens Gustedt, May 02 '15 at 13:24
`valgrind` tells me, that `Conditional jump or move depends on uninitialised value(s)` and that I have `Use of uninitialised value of size 8` for every line I have such a `printf()` as above. I also don't know, what an allocator function is. — J0hj0h, May 02 '15 at 13:25
That what I'd guessed. Have a look at this question: http://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope — Mat, May 02 '15 at 13:28

score 3 · Accepted Answer · answered May 02 '15 at 13:29

3

In your dn_new() you have:

if (dataPtr != NULL) {
    struct DynamicArray array = { 0, capacity, dataPtr };
    return &array;
}

Here, array is a local variable; it will be out of scope after returning it. You should allocate memory on the heap for that:

struct DynamicArray *array = malloc(sizeof *array);
array->size = 0;
array->capacity = capacity;
array->data = dataPtr;
return array;

And remember to free() this memory in your destructor function (dn_destroy()).

answered May 02 '15 at 13:29

user12205

2,684
1
20
40

Thanks a lot! I tried that before, but the compiler said `excess elements in scalar initializer`, so I tried to do a workaround. Apparently I did something differently than this... – J0hj0h May 02 '15 at 13:33
Yup, this is the problem here. By returning a local variable, it will get clobbered by future function calls. – nneonneo May 02 '15 at 13:33

Jens Gustedt · Answer 2 · 2015-05-02T13:28:34.090

2

It seems that you mix up how you interpret your sizes and capacities. You use them as if the count the number of elements but you allocate them (with realloc) as number of bytes. Change your code to the following:

int *dataPtr = realloc(a->data, 2*a->capacity*sizeof(a->data[0]));

edited May 02 '15 at 13:28

answered May 02 '15 at 13:26

Jens Gustedt

76,821
6
102
177

I changed that (to sizeof(int), since we only allow int values), but the problem described above still exists. Thanks a lot for that hint, though! – J0hj0h May 02 '15 at 13:28

Copying component of struct deletes another component of the same struct

2 Answers2