C Void ending issue

Question

I am trying to write a simple parking arrangement code, I want to sort the capacity by 1000 vehicles, color, license plate and model

#include <stdio.h>
#include <stdlib.h>
void NewCar()
{
    char model[1000][20];
    char color [1000][20];
    char number[1000][20];
    int x = 1;
        printf("\nModel: ");
        scanf("%s",model[x]);
        printf("Color: ");
        scanf("%s",color[x]);
        printf("Number: ");
        scanf("%s",number[x]);
}
void CarList()
{
    int x;
    char model[1000][20];
    char color [1000][20];
    char number[1000][20];
    for (x ; x >= 1 ; x--)
    {
        printf("\n%d. Car: %s %s %s",x,number[x],model[x],color[x]);
    }
}
int main()
{
    char model[1000][20];
    char color [1000][20];
    char number[1000][20];
    char menu;
    int x = 1;
    flag:
    printf("New Car(N)\nCar List(L)\n");
    scanf("%s",&menu);
    if (menu == "n" || menu == "N")
    {
        NewCar();
        goto flag;
    }
    if (menu == "l" || menu == "L")
    {
        CarList();
        goto flag;
    }
}

when i don't use void, the code works but i have to use void

Example of the output I want;

 1. Car Red Jeep FGX9425
 2. Car Yellow Truck OKT2637
 3. Car Green Sedan ADG4567
 ....

What do you mean by "use void", exactly? And what doesn't work? And in what way? Your question is very unclear. — Asteroids With Wings, Apr 12 '20 at 22:34
Please don't use `goto` as a substitute for a loop. It's a really bad idea — UnholySheep, Apr 12 '20 at 22:35
@UnholySheep It's not _that_ bad. In this particular case (a state machine) it may even be clearest. — Asteroids With Wings, Apr 12 '20 at 22:35
`scanf("%s",&menu);` is very wrong as `menu` is a single `char` and thus cannot store any string except the empty one. Probably meant to use `%c` instead. — kaylum, Apr 12 '20 at 22:38
There's a lot of issues in this code unrelated to `void`, e.g.: `scanf("%s",&menu);` is wrong, as `menu` is a single `char` and `if (menu == "n" || menu == "N")` is not how you compare strings in C (you'd use `strcmp` for that) — UnholySheep, Apr 12 '20 at 22:38
@UnholySheep there is nothing wrong with using goto as long as you use it correctly. — Fredrik, Apr 12 '20 at 22:44
You're using "parallel" arrays (e.g. `color`, `model`, and `number`) that are all indexed by the same thing. This "cries out" for a `struct`: (e.g.) `struct car { char color[20]; char model[20]; char number[20]; }; struct car carlist[1000];`You are repeating these arrays at _function_ scope, so changes in one will _never_ be seen by another function (e.g. `CarList` will never see changes to the values in `main`). Better is: `void CarList(struct car *list,int carcount) { ... }` and have `main` do: `CarList(&carlist,carcount);` where `carcount` is the # of active/valid elements in `carlist` — Craig Estey, Apr 12 '20 at 22:46
[Always] compile with `-Wall -O2` to get warnings and _fix_ those warnings/errors. — Craig Estey, Apr 12 '20 at 22:53

Craig Estey · Accepted Answer · 2020-04-13T16:06:53.220

This is prefaced by my top comments.

Never use goto. Use (e.g.) a while loop.

Your scanf for menu would [probably] overflow.

As others have mentioned, a number of bugs.

I've refactored your code with your old code and some new code. This still needs more error checking and can be generalized a bit more, but, I've tested it for basic functionality:

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

// description of a car
struct car {
    char model[20];
    char color[20];
    char number[20];
};

int
NewCar(struct car *cars,int carcount)
{
    struct car *car = &cars[carcount];

    printf("\nModel: ");
    scanf("%s", car->model);

    printf("\nColor: ");
    scanf("%s", car->color);

    printf("\nNumber: ");
    scanf("%s", car->number);

    ++carcount;

    return carcount;
}

void
CarList(struct car *cars,int carcount)
{
    struct car *car;
    int caridx;

    for (caridx = 0;  caridx < carcount;  ++caridx) {
        car = &cars[caridx];
        printf("%d. Car: %s %s %s\n",
            caridx + 1, car->number, car->model, car->color);
    }
}

int
main(int argc,char **argv)
{
#if 1
    int carcount = 0;
    struct car carlist[1000];
#endif
#if 0
    char menu;
    int x = 1;
#else
    char menu[20];
#endif

    // force out prompts
    setbuf(stdout,NULL);

    while (1) {
        printf("New Car(N)\nCar List(L)\n");
#if 0
        scanf("%s", &menu);
#else
        scanf(" %s", menu);
#endif

        // stop program
        if ((menu[0] == 'q') || (menu[0] == 'Q'))
            break;

        switch (menu[0]) {
        case 'n':
        case 'N':
            carcount = NewCar(carlist,carcount);
            break;

        case 'l':
        case 'L':
            CarList(carlist,carcount);
            break;
        }
    }

    return 0;
}

UPDATE:

As you said, there are a few minor errors, it's not a problem for me, but I can write errors if you want to know and fix them.(if you write the plate with a space between it, the code repeats the "new car car list" command many times)

Okay, I've produced an enhanced version that replaces the scanf with a function askfor that uses fgets. The latter will prevent [accidental] buffer overflow. And, mixing scanf and fgets can be problematic. Personally, I always "roll my own" using fgets as it can provide finer grain control [if used with wrapper functions, such as the askfor provided here]

Edit: Per chux, I've replaced the strlen for removing newline with a safer version that uses strchr:

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

#define STRMAX      20

// description of a car
struct car {
    char model[STRMAX];
    char color[STRMAX];
    char number[STRMAX];
};

// askfor -- ask user for something
void
askfor(const char *tag,char *ptr)
{

    printf("Enter %s: ",tag);
    fflush(stdout);

    fgets(ptr,STRMAX,stdin);

    // point to last char in buffer
    // remove newline
#if 0
    ptr += strlen(ptr);
    --ptr;
    if (*ptr == '\n')
        *ptr = 0;
#else
    // remove trailing newline [if it exists]
    ptr = strchr(ptr,'\n');
    if (ptr != NULL)
        *ptr = 0;
#endif
}

int
NewCar(struct car *cars,int carcount)
{
    struct car *car = &cars[carcount];

    askfor("Model",car->model);
    askfor("Color",car->color);
    askfor("Number",car->number);

    ++carcount;

    return carcount;
}

void
CarList(struct car *cars,int carcount)
{
    struct car *car;
    int caridx;

    for (caridx = 0;  caridx < carcount;  ++caridx) {
        car = &cars[caridx];
        printf("%d. Car: %s %s %s\n",
            caridx + 1, car->number, car->model, car->color);
    }
}

int
main(int argc,char **argv)
{
    int carcount = 0;
    struct car carlist[1000];
    char menu[STRMAX];

    // force out prompts
    setbuf(stdout,NULL);

    while (1) {
        askfor("\nNew Car(N)\nCar List(L)",menu);

        // stop program
        if ((menu[0] == 'q') || (menu[0] == 'Q'))
            break;

        switch (menu[0]) {
        case 'n':
        case 'N':
            carcount = NewCar(carlist,carcount);
            break;

        case 'l':
        case 'L':
            CarList(carlist,carcount);
            break;
        }
    }

    return 0;
}

UPDATE #2:

Thank you for your bug fix, but as I said in my question, I have to do the "New car" feature using void. You did it with int, can you do it with void?

Okay. When you said "using void", what you meant wasn't totally clear to me [or some others]. There were enough bugs that they overshadowed some other considerations.

So, I have to assume that "using void" means that the functions return void.

Your original functions were defined as void NewCar() and void CarList(). Those could not have done the job as is, so they had to be changed.

If you have similar criteria, a better way to phrase that would be:

I must create two functions, with the following function signatures ...

Anyway, here's the updated code:

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

#define STRMAX      20

// description of a car
struct car {
    char model[STRMAX];
    char color[STRMAX];
    char number[STRMAX];
};

// askfor -- ask user for something
void
askfor(const char *tag,char *ptr)
{

    printf("Enter %s: ",tag);
    fflush(stdout);

    fgets(ptr,STRMAX,stdin);

    // remove trailing newline [if it exists]
    ptr = strchr(ptr,'\n');
    if (ptr != NULL)
        *ptr = 0;
}

void
NewCar(struct car *cars,int *countptr)
{
    int carcount = *countptr;
    struct car *car = &cars[carcount];

    askfor("Model",car->model);
    askfor("Color",car->color);
    askfor("Number",car->number);

    carcount += 1;
    *countptr = carcount;
}

void
CarList(struct car *cars,int carcount)
{
    struct car *car;
    int caridx;

    for (caridx = 0;  caridx < carcount;  ++caridx) {
        car = &cars[caridx];
        printf("%d. Car: %s %s %s\n",
            caridx + 1, car->number, car->model, car->color);
    }
}

int
main(int argc,char **argv)
{
    int carcount = 0;
    struct car carlist[1000];
    char menu[STRMAX];

    // force out prompts
    setbuf(stdout,NULL);

    while (1) {
        askfor("\nNew Car(N)\nCar List(L)",menu);

        // stop program
        if ((menu[0] == 'q') || (menu[0] == 'Q'))
            break;

        switch (menu[0]) {
        case 'n':
        case 'N':
#if 0
            carcount = NewCar(carlist,carcount);
#else
            NewCar(carlist,&carcount);
#endif
            break;

        case 'l':
        case 'L':
            CarList(carlist,carcount);
            break;
        }
    }

    return 0;
}

However, given your original functions, it may be possible that the signatures have to be: void NewCar(void) and void CarList(void) and that the car list variables must be global scope.

This would be a less flexible and desirable way to do things, but here is a version that uses only global variables for the lists:

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

#define STRMAX      20

// description of a car
struct car {
    char model[STRMAX];
    char color[STRMAX];
    char number[STRMAX];
};

#if 1
int carcount = 0;
struct car carlist[1000];
#endif

// askfor -- ask user for something
void
askfor(const char *tag,char *ptr)
{

    printf("Enter %s: ",tag);
    fflush(stdout);

    fgets(ptr,STRMAX,stdin);

    // remove trailing newline [if it exists]
    ptr = strchr(ptr,'\n');
    if (ptr != NULL)
        *ptr = 0;
}

void
NewCar(void)
{
    struct car *car = &carlist[carcount];

    askfor("Model",car->model);
    askfor("Color",car->color);
    askfor("Number",car->number);

    carcount += 1;
}

void
CarList(void)
{
    struct car *car;
    int caridx;

    for (caridx = 0;  caridx < carcount;  ++caridx) {
        car = &carlist[caridx];
        printf("%d. Car: %s %s %s\n",
            caridx + 1, car->number, car->model, car->color);
    }
}

int
main(int argc,char **argv)
{
#if 0
    int carcount = 0;
    struct car carlist[1000];
#endif
    char menu[STRMAX];

    // force out prompts
    setbuf(stdout,NULL);

    while (1) {
        askfor("\nNew Car(N)\nCar List(L)",menu);

        // stop program
        if ((menu[0] == 'q') || (menu[0] == 'Q'))
            break;

        switch (menu[0]) {
        case 'n':
        case 'N':
#if 0
            carcount = NewCar(carlist,carcount);
#else
            NewCar();
#endif
            break;

        case 'l':
        case 'L':
#if 0
            CarList(carlist,carcount);
#else
            CarList();
#endif
            break;
        }
    }

    return 0;
}

Thank you very much for your answer, I marked it as the correct answer — AomineDaici, Apr 12 '20 at 23:14
You're welcome. You can also "upvote" it as a "good" answer (and can upvote other answer that may come it). Good luck. BTW, a `struct` is _very_ powerful, so it's worth some study. — Craig Estey, Apr 12 '20 at 23:17
As you said, there are a few minor errors, it's not a problem for me, but I can write errors if you want to know and fix them.(if you write the plate with a space between it, the code repeats the "new car car list" command many times) — AomineDaici, Apr 12 '20 at 23:18
`scanf(" %s", menu);` is as good a [`gets()`](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used). Recommend a width limit. ``scanf("%19s", menu);` — chux - Reinstate Monica, Apr 12 '20 at 23:30
@chux-ReinstateMonica Did a 2nd code example that replaces `scanf` with `fgets`, both for preventing buffer overflow and getting whole line strings. I'm not as much of a fan of `scanf("%19s",menu)` and I tried something similar to `int max = 19; printf("%*s\n",max,menu);` for `scanf`: `int max = sizeof(menu) - 1; scanf("%*s",max,menu);` but it didn't work. Is there such an equivalent for `scanf`? I'm looking for something other than: https://stackoverflow.com/questions/25410690/scanf-variable-length-specifier or use of the `m` specifier [as it does an extra/hidden `malloc`] — Craig Estey, Apr 13 '20 at 01:12
Is there such an equivalent for scanf? ---> No - sadly. `m` is not part of the standard C lib. Could use `#define xstr(a) str(a) #define str(a) #a #define STRLEN_MAX 19 char menu[STRLEN_MAX + 1]; scanf("%" xstr(STRLEN_MAX) "s", menu);`, but `fgets()` is better. — chux - Reinstate Monica, Apr 13 '20 at 01:49
Minor: `ptr += strlen(ptr); --ptr; if (*ptr == '\n') *ptr = 0;` is UB with nefarious input and `*ptr== '\0';` See [here](https://stackoverflow.com/a/27729970/2410359) and others. — chux - Reinstate Monica, Apr 13 '20 at 01:52
@chux-ReinstateMonica _Sigh_, yes. I fixed it. Don't know what I was [non-]thinking as I mostly use `strchr` [vs. `strlen`] for newline stripping in my own code, for the reasons alluded to ... — Craig Estey, Apr 13 '20 at 02:16
Nit -- "*never use*" `goto` -- except to jump out of nested loops, etc.. It's not a blanket *never*. It does have it's uses. GNU libc would fall apart without it. — David C. Rankin, Apr 13 '20 at 02:17
@DavidC.Rankin The [linux] kernel uses `goto` [probably] more than `glibc`. I used `goto` for the same purpose [_four_ decades ago ;-)]. Better a hard rule for a _newbie_ [with the bad habit of using `goto` instead of loop]. IMO, `goto` is an _advanced_ technique. But, even though _slightly_ slower, instead of `while (1) { ... while (1) { ... if (stop_on_expr) goto stopme; ... }} stopme:`, I use a control variable: `int more = 1; while (more) { ... while (more) { ... if (stop_on_expr) { more = 0; break; } } }` — Craig Estey, Apr 13 '20 at 02:30
@craigestey Thank you for your bug fix, but as I said in my question, I have to do the "New car" feature using void. You did it with int, can you do it with void? — AomineDaici, Apr 13 '20 at 12:03

C Void ending issue

1 Answers1

Linked