Having problems with 2D char arrays

Question

So I've got an assignment where my program asks the brand (10 letters), model (10 letters), age (1986 - 2019) and cost (positive real number) of 10 cars and then wants the program to check which car is the oldest and to print out it's brand and model. I don't have a problem with the first part but with the second part. The code is:

//First part
    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    #define C 10
    #define M 11

    int main(void)
    {   

        char brand[C][M];
        char model[C][M];
        int year[C];
        float cost[C];
        int i, len1, len2, min;

for(i=0; i<C; i++){
        printf("Car %d\n", i+1);

        do{
        printf("Brand: ");
        scanf("%s", brand[i]);
        len1 = strlen(brand[i]);
        } while(len1<0 || len1>10);

        do{
        printf("Model: ");
        scanf("%s", model[i]);
        len2 = strlen(model[i]);
        } while(len2<0 || len2>10);

        do{
            printf("Year: ");
            scanf("%d", &year[i]);
        } while(year[i]<1986 || year[i]>2019);

        do{
            printf("Cost: ");
            scanf("%d", &cost[i]);
        } while(cost[i]<=0);
}
//Second part
    year[0] = min;
    for(i=0; i<10; i++)
        if(year[i] < min){
            min = year[i];
            printf("\nThe oldest car is %s %s\n", brand[i], model[i]);
        }

For some reason it either prints out gibberish in the place of brand[i] or if I lose the columns of the if statement prints out all the car brands and their models, where I only want the oldest one.

Please post a [Minimal, Reproducible Example](https://stackoverflow.com/help/minimal-reproducible-example), not just a snippet. It is unclear what data type all of your variables have, which one the 2D array is (if any) and whether the loop is not going out of bounds. — Blaze, Dec 03 '19 at 11:09
From what you are describing your problem is not in the snippet of code you posted but elsewhere. Also i think you wanted to have `min = year[0]` instead of `year[0] = min` — John Doe, Dec 03 '19 at 11:10
@ThanosMitropoulos modify your question yo include a [Minimal, Reproducible Example](https://stackoverflow.com/help/minimal-reproducible-example) so that everyone could easily find all the information instead of putting them in comments. — John Doe, Dec 03 '19 at 11:11
@JohnSmith When I change it to `min = year[0]` the program prints nothing — Thanos Mitropoulos, Dec 03 '19 at 11:12
as I said, we need a [Minimal, Reproducible Example](https://stackoverflow.com/help/minimal-reproducible-example) to help you, the code snippet you shared doesn't have any problems on its own (except `min` not being initialized and you not telling us how `year` is defined) — John Doe, Dec 03 '19 at 11:14
assuming your code is correct if it doesn;t print anything when you change it to `min = year[0]` it means that the first year in the array is the oldest one since you only check for `<`. Anyway since you say that it prints out gibberish you probably have a problem somewhere else in your code but we can't help you if you don't show it to us. — John Doe, Dec 03 '19 at 11:17
Never ever do: `scanf("%s", ...` It allows buffer overflow. And this check `while(len1<0 || len1>10);` is too late... — Support Ukraine, Dec 03 '19 at 11:23
@JohnSmith Yeah but shouldn't it print at least the car of that year? — Thanos Mitropoulos, Dec 03 '19 at 11:23
@4386427 should i use gets instead? and what do you mean too late? — Thanos Mitropoulos, Dec 03 '19 at 11:24
@ThanosMitropoulos no because you print it only if it's lower, but you just assigned it so it's equal not lower — John Doe, Dec 03 '19 at 11:27
No **not** `gets` !! Use `fgets` And by too late I mean: If `len1` is greater than 10, you already have a buffer overflow. Use `fget` as it allows you to set a maximum of characters to read in a very simple way — Support Ukraine, Dec 03 '19 at 11:27
The code isn't complete... Make sure to make a copy of all your code — Support Ukraine, Dec 03 '19 at 11:28
Use scanf, if you want, but restrict the length of chars written to your pointers like: `scanf("%11s", brand + i);` , and if you wanna be paranoid, ensure a terminal 0: `scanf("%10s", brand[i]); brand[i][10] = 0;` Do this for any string you read via `scanf` . — Michael Beer, Dec 03 '19 at 11:28
Code indentation is off but it seems to me that "second part" is inside the `for(i=0; i — Support Ukraine, Dec 03 '19 at 11:30

John Doe · Accepted Answer · 2019-12-03T12:17:20.613

Aside from scanf not being recommended there are some problems with this code, first when you read the brand and model you do:

do{
    printf("Brand: ");
    scanf("%s", brand[i]);
    len1 = strlen(brand[i]);
} while(len1<0 || len1>10);

The problem here is that you first write the string to brand[i] and then check if it's too long, but you have already written it into the array so if the string is longer than your space you already have a buffer overflow. Limit the size you can read with scanf using scanf("%10s, brand[i]) or better yet use fgets(brand[i], sizeof(brand[i]), stdin).

Next in the second part you use min without initializing it, and you overwrite the content of year[0] with it. You probably wanted something like:

min = 2020; // or a number that will be bigger than all your cars anyway
int older = 0; 
i = 0;
for(i=0; i<C; i++){ // Use C here, you have it might as well use it instead of magic numbers
    if(year[i] < min){
        older = i;
        min = year[i];
    }
}
printf("\nThe oldest car is %s %s\n", brand[older], model[older]);

but bare in mind that this solution will print multiple cars if they are the oldest ones and have the same year

Comments are not for extended discussion; this conversation has been [moved to chat](https://chat.stackoverflow.com/rooms/203553/discussion-on-answer-by-john-smith-having-problems-with-2d-char-arrays). — Samuel Liew, Dec 03 '19 at 19:49

Having problems with 2D char arrays

1 Answers1