Struct and char**

Question

I am completely new in C programming recently I started looking a bit more into pointer and I have encountered a problem with my code.

So I try to pass a string to a char** that is into a struct but I am always getting errors. Thanks in advance here is some code

This is the struct I used.

typedef struct Queue {
int capacity;
int front;
int rear;
int size;
char **car_id;
};

And this is what I try to put into it. Inside the first if you can see I try to strcpy but fails. Even if I put & or *.

    printf("Printing the contect of the TXT file\n");
int countS=-1,countQ=-1;
char line[10];
FILE *fp;
fp = fopen("car.txt","r");
if (fp == NULL) {
    printf("Error: File doesn't exist exiting the programm!");
    exit(1);
}
while(!feof(fp)){
      fgets(line,10,fp);
      printf("%s",line);

       if(line[(strlen(line))-2]=='Q'){
        countQ++;
        line[strlen(line)-2] = 0;
        strcpy(car_id,line);
        strcpy(QBoat->car_id[countQ],car_id);
        QBoat->rear=countQ;
        QBoat->front=0;
        QBoat->size=countQ;
       }else if(line[(strlen(line))-2]=='S'){
        countS++;
        line[strlen(line)-2] = 0;
        SBoat->top = countS;
        //strcpy(SBoat->car_id[countS],line);
       }

}
fclose(fp);

Please don't loop over `feof()` - http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong — , Feb 01 '17 at 20:58
Yes. where you `malloc()` it. Post a complete code sample that will compile and exhibit the problem, — , Feb 01 '17 at 21:14

Schwern · Answer 1 · 2017-02-02T19:38:27.980

It's very easy to mess up memory allocation and deallocation with a struct. So it's best to always write new and destroy functions for every struct.

/* To avoid confusion, it's best to declare structs as type definitions
   and name them Something_t. Capital letter to avoid conflict with
   built in types, _t to note that it is a type, and to avoid conflict
   with other names. */
typedef struct {
    /* Using size_t means less type casting */
    size_t capacity;
    int front;
    int rear;
    int size;
    char **items;
} Queue_t;

Queue_t *Queue_new( const size_t capacity ) {
    /* Use calloc because we have to zero everything anyway */
    Queue_t *queue = calloc( 1, sizeof(Queue_t) );

    /* Allocate enough space to store a list of whatever
       queue->items is a list of */
    queue->capacity = capacity;
    queue->items = malloc( capacity * sizeof(*(queue->items)) );

    return queue;
}

void Queue_destroy( Queue_t *queue ) {
    /* Since items were copied into the list, it's the queue's responsibility
       to free them */
    for( int i = queue->front; i < queue->rear; i++ ) {
        free( queue->items[i] );
    }

    /* Now free the list of items */
    free( queue->items );

    /* And finally the struct itself */
    free( queue );
}

Now that the struct has been allocated, and its list of items, you have to make sure not to add too many items. Your code adding to the queue never checks if it's overrun the queue's capacity. For this reason, it's best to write a function to add items that will correctly check capacity.

void Queue_add( Queue_t *queue, const char *item ) {
    if( queue->size >= (int)queue->capacity ) {
        fprintf(stderr, "Queue capacity of %zu exceeded!\n", queue->capacity);
        exit(1);
    }

    /* You used strcpy in your example, so I'm following that.
       You could also not copy and store the original pointer. */
    queue->items[ queue->rear ] = strdup( item );

    /* I don't know if this is right */
    queue->rear++;
    queue->size++;
}

The queuing logic might be wrong, I'm not good with queues, but you get the idea. You could even later extend the queue to automatically resize itself.

Now you can test those work in isolation. Once you're sure they work, you can try reading from a file and using your queue functions.

int main() {
    char filename[] = "car.txt";
    FILE *fp = fopen(filename,"r");
    if (fp == NULL) {
        fprintf(stderr, "Couldn't read '%s': %s\n", filename, strerror(errno));
        exit(1);
    }

    /* Just picked a number out of thin air */
    Queue_t *qboats = Queue_new(256);
    Queue_t *sboats = Queue_new(256);

    char line[10];
    while( fgets( line, 10, fp) != NULL ) {
        size_t len = strlen(line);

        /* Choose the right queue to use */
        Queue_t *queue;
        switch ( line[len-2] ) {
            case 'S':
                queue = sboats;
                break;
            case 'Q':
                queue = qboats;
                break;
            default:
                fprintf( stderr, "Can't understand line '%s'\n", line );
                continue;
                break;
        }

        /* Chop off the identifier */
        line[len - 2] = '\0';

        /* Add to the queue */
        Queue_add( queue, line );
    }

    /* Do something with the queues. This should probably be Queue_print(). */
    for( int i = qboats->front; i < qboats->rear; i++ ) {
        printf("qboat: %s\n", qboats->items[i]);
    }

    for( int i = sboats->front; i < sboats->rear; i++ ) {
        printf("sboat: %s\n", sboats->items[i]);
    }

    /* Now clean them up */
    Queue_destroy( sboats );
    Queue_destroy( qboats );
}

Most of the work is now determining which queue to use.

With code like `queue->items = malloc( capacity * sizeof(char *) );`, I find `queue->items = malloc(capacity * sizeof *(queue->items) );` easier to code, review and maintain. — chux - Reinstate Monica, Feb 02 '17 at 14:02

Struct and char**

1 Answers1