Sili Lai Sili Lai - 4 months ago 9
C Question

Why the output strings concatenate in a weird way?

I want to use my generic vector to hold the data from a line in the csv file. But I meet a problem. My input is:


MonsterID,Name,Hitpoints,Attack,Defence,AttackTimes,Gold,Exp,Special


The output become

Field 0 would be MonsterIName
Field 1 would be Name
Field 2 would be HitpointAttack
Field 3 would be Attack
Field 4 would be Defence
Field 5 would be AttackTiGold
Field 6 would be Gold
Field 7 would be Exp
Field 8 would be Special


Here are the parts of the code relevant to this problem:

Definition of the
Vector
type:

typedef void (*VectorFreeFunction)(void *element);

typedef struct Vector {
void *elements; // An array of elements
int capacity; // The allocated size of the array
int size; // The number of elements in use
size_t elemSize; // Size of the data type of the element.
VectorFreeFunction freeFunc;
} Vector;


csv.c

...
Vector *getfields(char *line) {
int i = 0;
const char *token;

Vector *fields = vectorAlloc(sizeof(char*), NULL);

for (token = strtok(line, ",");
token && *token;
token = strtok(NULL, ",\n"), i++)
{
vectorPush(fields, (const void*)token);
}

for (int i = 0; i != fields->size; i++) {
printf("Field %d would be %s\n", i, (const char*)vectorAt(fields, i));
}

return fields;
}


vector.c

Vector *vectorAlloc(size_t elemSize, VectorFreeFunction freeFunc) {
Vector *vector = malloc(sizeof(Vector));
if (vector == NULL) {
fatalError("Cannot allocate vector");
}

vector->capacity = DEFAULT_CAPACITY;
vector->size = 0;
vector->elemSize = elemSize;

vector->elements = malloc(elemSize * vector->capacity);
if (vector->elements == NULL) {
fatalError("Cannot allocate elements array of the vector");
}

vector->freeFunc = freeFunc;
return vector;
}

void *vectorAt(Vector *vector, int position) {
assert(position < vector->size && position >= 0);
return ((char*)vector->elements + (position * vector->elemSize));
}

void vectorPush(Vector *vector, const void *element) {

if (vector->size == vector->capacity) {
_vectorDoubleCapacity(vector);
}

void *destAddr = (char*)vector->elements + vector->size * vector->elemSize;
memcpy(destAddr, element, vector->elemSize); // add to end of vector

vector->size++;
}

void _vectorDoubleCapacity(Vector *vector) {
vector->capacity *= 2;

vector->elements = realloc(vector->elements, vector->elemSize * vector->capacity);
if (vector->elements == NULL) {
fatalError("Resizing the capacity of vector fails");
}
}

Answer

The elements you push onto the Vector are pointers into the array pointed to by the line argument to getfields. It is difficult to keep track of the lifespan of the Vector elements this way, you should instead allocate copies of the tokens with strdup() and pass free as a VectorFreeFunction when creating the Vector.

Futhermore, vectorAt returns a pointer to the vector element, which itself is a char *, the way you use it to print the contents of the Vector is incorrect: you should cast it as (char **) and dereference it.

Here is a corrected version:

Vector *getfields(char *line) {
    int i = 0;
    const char *token;

    Vector *fields = vectorAlloc(sizeof(char*), free);

    for (token = strtok(line, ",");
         token && *token;
         token = strtok(NULL, ",\n"), i++) {
        vectorPush(fields, strdup(token));
    }

    for (int i = 0; i < fields->size; i++) {
        printf("Field %d would be %s\n", i, *(char **)vectorAt(fields, i));
    }

    return fields;
}