How to copy value into array element correctly?

Question

I need an universal function that will put a value into array, I wrote it so:

int add_element(void** arr, void* val, int t_size, int* size, int* capacity) {

    if ((*size) == (*capacity)) {
        *capacity += ARRAY_INC;
        *arr = (char*)realloc(*arr, (*capacity) * sizeof(char));

        if (*arr == NULL) {
            return 1;
        }
    }

    memcpy(*arr + ((*size)++), val, t_size);

    return 0;
}

It works perfect for a char array but I have troubles with array of structures in the next code:

int scan_tokens(Token** p_tokens, int* size) {
    char* line;
    int len;

    if (get_line(&line, &len)) {
        return 1;
    }

    int capacity = ARRAY_INC;
    *size = 0;
    *p_tokens = (Token*)malloc(capacity * sizeof(Token));
    int start = -1;

    for (int i = 0; i < len; ++i) {
        char ch = line[i];

        if (isdigit(ch)) {

            if(start == -1) {
                start = i;
            }
        } else {
            if (start != -1) {
                Token t;
                int count = i - start;
                t.size = count;
                t.data.token = (char*)malloc(count * sizeof(char));
                memcpy(t.data.token, line + start, count * sizeof(char));
                start = -1;
                add_element(p_tokens, &t, sizeof(Token), size, &capacity);
            }

            Token t;

            switch(ch) {
                case SUM:
                case SUB:
                case MUL:
                case DIV:
                case LBR:
                case RBR:
                    t.size = 0;
                    t.data.ch = ch;
                    add_element(p_tokens, &t, sizeof(Token), size, &capacity);
                    break;
                default:
                    return 1;
            }
        }
    }

    if (start != -1) {
        Token t;
        int count = len - start;
        t.size = count;
        t.data.token = (char*)malloc(count * sizeof(char));
        memcpy(t.data.token, line + start, count * sizeof(char));
        start = -1;
        // Will a copy of `t` in p_tokens be reseted on the next iteration?
        add_element(p_tokens, &t, sizeof(Token), size, &capacity);
    }

    free(line);
    line = NULL;

    return 0;
}

The problem is that added element of array was reset on the next iteration of loop. And I can't get why? When I call memcpy in add_element it must copy all fields of Token structure in related fields of array element structures, it isn't?
What do I doing wrong and how to fix? I can't get already...

FIXED SECONDARY ERROR

int add_element(void** arr, void* val, int t_size, int* size, int* capacity) {

    if ((*size) == (*capacity)) {
        *capacity += ARRAY_INC;
        *arr = realloc(*arr, (*capacity) * t_size);

        if (*arr == NULL) {
            return 1;
        }
    }

    memcpy(*arr + ((*size)++), val, t_size);

    return 0;
}

The code still has same problem.
ADDED
Seems I got my error, it's here *arr + ((*size)++): arr is void** so maybe (*arr + some_num) will give a wrong offset but I don't know how to fix it.

capacity means capacity in terms of "nr of elements", not capacity in terms of bytes, right? Then something like (*capacity) * sizeof(char) is suspicious... — Stephan Lechner
– Stephan Lechner, Commented Oct 1, 2017 at 21:39
@StephanLechner, thank you very much, I didn't notice when I rewrote from char. — Denis Steinman
– Denis Steinman, Commented Oct 1, 2017 at 21:42
@xing, arr is a pointer to the pointer of array. I.e. *arr is a pointer to the first element of array. — Denis Steinman
– Denis Steinman, Commented Oct 1, 2017 at 22:02
Also *arr + is an error, you cannot do arithmetic on void * — M.M
– M.M, Commented Oct 1, 2017 at 22:05
Yes you can cast to char*. But the aliasing problem still remains. It'd be better to accept void * argument and return the change value as void * — M.M
– M.M, Commented Oct 1, 2017 at 22:17

Some programmer dude · Accepted Answer · 2017-10-01 21:41:02Z

In scan_tokens you have the initial allocation:

*p_tokens = malloc(capacity * sizeof(Token));

_{[Unnecessary cast removed]}

Here you allocate capacity * sizeof(Token) bytes.

Then in add_element you have:

*arr = realloc(*arr, (*capacity) * sizeof(char));

Here you allocate *capacity bytes.

This is of course not correct and will most likely allocate to little. In the add_element function you should allocate a multiple of t_size bytes:

*arr = realloc(*arr, (*capacity) * t_size);

On a very related note: Don't reassign back to the pointer you pass to the realloc function. If realloc fails it will return a null pointer, leading to you losing the original pointer. Use a temporary variable that you check before assigning.

Thank you very much, it's really my error... But it still doesn't resolve my problem

Denis Steinman · Accepted Answer · 2017-10-01 22:15:14Z

Thank you all for help, I got my error. Below a correct variant of my function:

int add_element(void** arr, void* val, int t_size, int* size, int* capacity) {

    if ((*size) == (*capacity)) {
        *capacity += ARRAY_INC;
        *arr = realloc(*arr, (*capacity) * t_size);

        if (*arr == NULL) {
            return 1;
        }
    }

    memcpy((char*)(*arr) + (*size) * t_size, (char*)val, t_size);
    ++(*size);

    return 0;
}

I was need to cast my void pointer to char pointer.

Collectives™ on Stack Overflow

How to copy value into array element correctly?

2 Answers 2

1 Comment

Comments

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Related