Pass a structure to a function and store values in the structure's element

Question

I want to pass a structure to a function and store value in the structure's element. Here is my code.

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

typedef struct {
    uint32_t len;
    uint16_t *arr;
} seq;

void write_seq(seq *q, int val)
{
    // How to implement this function?

    uint16_t *tmp;
    tmp = (uint16_t *)malloc(sizeof(uint16_t));
    *tmp = val;

    *(q->arr + q->len) = *tmp;
    q->len += 1;

}

int main(int argc, char *argv[])
{
    seq q;
    q.len = 0;
    q.arr = NULL;
    int i;
    for (i = 0; i < 10; i++) {
        write_seq(&q, i);
    }

    printf("length is %d\n", q.len);
    for (i = 0; i < q.len; i++) {
        printf("%d\n", *(q.arr+i));

    }

    return 0;
}

I want to write 0 to 9 to a memory block which q.arr points to.
I don't want to use malloc in main(), since I don't know how many bytes I need before I call write_seq. I want to locate new memory every time when write_seq is called. Here's how output should look like.

length is 10
0
1
2
3
4
5
6
7
8
9

My implementation of write_seq() would cause core dumped. I don't know how to fix it. Thank you.

Answer 1

You need to use realloc(), not malloc(), like this:

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

typedef struct {
    int len;
    int *arr;
} seq;

void write_seq(seq *q)
{
    q->arr = realloc (q->arr, (q->len + 1) *  sizeof(int));
    q->arr[q->len] = q->len;
    q->len++;
}

int main(void)
{
    seq q;
    q.len = 0;
    q.arr = NULL;

    for(int i = 0; i < 10; ++i)
        write_seq(&q);


    printf("length is %d\n", q.len);
    for (int i = 0; i < q.len; i++) {
        printf("%d\n", q.arr[i]);
    }

    free(q.arr);

    return 0;
}

Output:

length is 10
0
1
2
3
4
5
6
7
8
9

Answer 2

To add members to the array when you don't know in advance the size of that array, you'd need to use realloc() to increase the size on demand. However it's inefficient to do that for every change to the array size, so it would be more usual to allow for a certain amount of headroom (whether that be a fixed increment or a percentage based amount) in the buffer.

The corollary of that is you need to store the current capacity of the buffer as well as the offset to the amount currently used.

This also means that there'll be a certain amount of memory wastage, but that's the trade off you have to make.

My approach would look something like this, abstracting the operations that you might want to perform on a seq.

typedef struct {
    size_t    capacity;
    size_t    offset;
    uint16_t *arr;
} seq;

static const size_t SEQ_INITIAL = 8;
static const size_t SEQ_INCREMENT = 8;

int seq_init(seq *seq) {
    assert(seq != NULL);         /* catch null seq */
    assert(seq->arr == NULL);    /* error to call on already init'd seq */
    seq->capacity = SEQ_INITIAL;
    seq->offset = 0;
    seq->arr = malloc(seq->capacity * sizeof(seq->arr[0]));
    return seq->arr == NULL ? -1 : 0;
}

static int seq_grow(seq *seq) {  /* private implementation detail */
    size_t new_capacity = seq->capacity + SEQ_INCREMENT;
    void *p = realloc(seq->arr, new_capacity * sizeof(seq->arr[0]));
    if (p == NULL) {             /* realloc failed, seq unmodified */
        return -1;
    }
    seq->arr = p;
    seq->capacity = new_capacity;
    return 0;
}

int seq_write(seq *seq, uint16_t value) {
    assert(seq != NULL);         /* catch null seq */ 
    if ((seq->offset == seq->capacity) && seq_grow(seq) < 0) {
        return -1;               /* write failed */
    }
    assert(seq->arr != NULL);    /* catch bad seq */
    assert(seq->offset < seq->capacity); /* ensure seq really has room */
    seq->arr[seq->offset++] = value;
    return 0;
}

void seq_free(seq *seq) {
    if (seq != NULL) {
        free(seq->arr);
        seq->arr = NULL;
    }
}