When accessing my graphic in my struct it prints an empty line

Question

Here is the struct that I am using.

#define MAX_CAR_LEN 12    ///< length does not include the trailing NUL byte


/// Racer_S structure represents a racer's row, position and display graphic.

    typedef struct Racer_S {

        int row;       ///< vertical row or "racing lane" of a racer

        int distance;  ///< column of rear of car, marking its position in race

        char *graphic; ///< graphic is the drawable text of the racer figure

    } Racer;

When I call this function everything works fine inside it and creates everything correctly. I am able to access the row and distance fine. When I try and print the graphic I am printed an empty row in my terminal. I believe that this might be because "graphic" in the struct is a char* but I assign it a fixed sized array of char. When this function is called and passed in the name "Tom", graphic is supposed to be "~O=Tom----o>". I am new to C what am I doing wrong?

Racer * make_racer( char *name, int row ){
    //Creating a newRacer instance
    Racer *newRacer = malloc(sizeof(*newRacer));
    newRacer->graphic = (char*)malloc(MAX_CAR_LEN);
    char car[MAX_CAR_LEN] = "~O=";     //"~O=-------o>"
    char *pCar;
    int length = strlen(name) + 2;
    //Add the name after the engine of the car
    for (int i = 0; i < length; ++i)
        car[i+3] = name[i];
    //Calculate the amount of dashes needed
    int printDashes = 7 - strlen(name);
    //add the extra dashes
    for (int j = 1; j <= printDashes; ++j)
        car[length + j] = '-';

    // creates the end of the car
    car[MAX_CAR_LEN-2] = 'o';
    car[MAX_CAR_LEN-1] = '>';
    pCar = strdup(car);

    newRacer->row = row;
    newRacer->distance = 0;
    newRacer->graphic = &car[0];
//    printf("%s\n", car);
    return newRacer;
}

This is the code I am running in my main to test it

Racer *t = make_racer("Tom", 4);
printf("%s\n", t->graphic);

You need to allocate memory for the string and copy it into that allocated memory. You're just pointing to a variable that immediately goes out of scope. You can probably use `strdup` or do it manually. I will say there's a lot of assumptions being made in your code about lengths. It would be very easy to access outside the bounds of the memory you've allocated. — Retired Ninja, Nov 14 '18 at 02:49
I added a new variable char *pCar = strdup(car); and made newRacer->graphic = pCar... but I am still seg faulting. hmmmmm — Jak, Nov 14 '18 at 03:04
Consider a [mcve] because the problem could very likely be in the code you have not shown, and we'd be able to see things like what MAX_CAR_LEN is. — Retired Ninja, Nov 14 '18 at 03:07
When you modify `car`, after adding `car[MAX_CAR_LEN-1] = '>';`, (which overwrites the final *nul-byte*) you fail to *nul-terminate* `car` by setting `car[MAX_CAR_LEN-1] = 0;` so you do not have a valid string in `car`, you simply have unterminated an *array of* `char`. When you then call `strdup(car);`, you invoke *Undefined Behavior* by passing an unterminated string to `strdup` which then happily runs off past the end of your array looking for the terminating character reading from memory you do not own... Fix `char car[MAX_CAR_LEN+1] = "~O=";` — David C. Rankin, Nov 16 '18 at 17:04

BEPP · Answer 1 · 2018-11-16T17:07:36.893

0

You have mentioned the below statement in your question.

This is the code I am running in my main to test it
acer *t = make_racer("Tom", 4);
printf("%s\n", t->graphic);

In make_racer function you have used a local charecter array variable called car and assigned the address to newRacer->graphic . This variable(char car[MAX_CAR_LEN+1] ;) memory goes out of scope after returning from the function.

Please refer this thread to understand more about the local scope in C.

Now to resolve your problem, In make_racer function you have to dynamically allocate memory for newRacer->graphic as well.

Racer * make_racer( char *name, int row ){
:
    Racer *newRacer = malloc(sizeof(*newRacer));
    newRacer->graphic = (char*)malloc(MAX_CAR_LEN+1);
:
    //newRacer->graphic = &car[0]; # this is wrong.
    strcpy(newRacer->graphic,car); //copy the content to allocated memory.

    /*
     * Warning:Just i am pointing out the issue here. 
     *    strcpy may cause buffer over run. 
     *    You have to use snprintf/strncpy family to prevent buffer overrun.
     */
}

Make sure you free the memory in your main.

int main() {
:
    free(newRacer->graphic);    //before free the momory for "newRacer"
    free(newRacer);
}

edited Nov 16 '18 at 17:07

answered Nov 14 '18 at 05:17

BEPP

875
1
12
36

I am still getting the error even with the fixes :/ – Jak Nov 14 '18 at 17:58
I have updated my code above for your reference. I would really appreciate it if you could check it over – Jak Nov 14 '18 at 18:01
@Jak, you have to copy the content to allocated memory. memory allocated for "car" will be freed up. please check my updated answer. – BEPP Nov 16 '18 at 16:54
@NaveenKumar - have a look at my comment under the question and see if you can fix the problem with `car` before `strdup` is called. Also `newRacer->graphic = &car[0];` is wrong because it assigns the address of an array created local to `make_racer()` which then goes out of scope on the function return. Seems like it should be `newRacer->graphic = pCar;` – David C. Rankin Nov 16 '18 at 17:10

When accessing my graphic in my struct it prints an empty line

1 Answers1