2

I'm messing around in C trying to learn a bit about how it works, and I ran into a problem. I have a structure defined with two character array variables. I'm populating them using fgets() from keyboard input. However when I go to print, the output looks like this:

Gibson
Les Paul
Fender
Stratocaster

When I really want it to look like this:

Gibson Les Paul
Fender Stratocaster

I can accomplish this just fine when using scanf opposed to fgets, but I figured I'd see if I can get some understanding on why this happens as I'm new to C.

Here is my code:

#include <stdio.h>

typedef struct Guitars
{ 
    char brand[10];
    char model[10];
} input;

void input_data(struct Guitars input[10])
{
    for(int i=0; i<2; i++)
    {
        printf("Please enter the brand: ");
        fgets(input[i].brand, 10, stdin);
        printf("Please enter the model: ");
        fgets(input[i].model, 10, stdin);
    }
}

void print_array(struct Guitars input[10])
{
    for(int i=0; i<2; i++) 
    {
        printf("%s%s", input[i].brand, &input[i].model);
    }   
}

int main(void) {
    struct Guitars input[10];
    input_data(input);
    print_array(input);
}

Thanks for any help!

  • Ensure that neither `.brand` nor `.model` contains a newline (but don't use `gets()`: see [Why `gets()` is too dangerous to be used — ever!](https://stackoverflow.com/questions/5431941/)). Then use `printf("%s %s\n", input[i].brand, input[i].model);` — noting the space between the `%s` conversion specifications and the newline at the end. You can make the break between brand and model clearer with a colon or dash or similar if you want. – Jonathan Leffler Mar 03 '19 at 18:51
  • Please don't vandalize your posts. – EJoshuaS - Stand with Ukraine Mar 03 '19 at 21:29

1 Answers1

2

Too big to fit in a single comment.

Ensure that neither .brand nor .model contains a newline (but don't use gets(): see Why gets() is too dangerous to be used — ever!). Remember that fgets() includes the newline (if it fits in the buffer). You can remove it with:

if (fgets(buffer, sizeof(buffer), stdin) != 0)
{
    buffer[strcspn(buffer, "\n")] = '\0';
    …
}

Then use:

printf("%s %s\n", input[i].brand, input[i].model);

Note the space between the %s conversion specifications and the newline at the end. You can make the break between brand and model clearer with a colon or dash or something similar if you want.

End of original comment.

Also, note that Stratocaster doesn't fit into an array of 10. Your structure needs bigger arrays to store your sample data. You also need to think about how many entries you make in the array — you've hard-wired 2 which is a bit restrictive.

Putting these changes together leads to code such as:

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

struct Guitars
{
    char brand[20];
    char model[20];
};

static int input_data(int max_entries, struct Guitars input[])
{
    int i;
    for (i = 0; i < max_entries; i++)
    {
        printf("Please enter the brand: ");
        if (fgets(input[i].brand, sizeof(input[i].brand), stdin) == 0)
        {
            fprintf(stderr, "Unexpected EOF\n");
            break;
        }
        input[i].brand[strcspn(input[i].brand, "\n")] = '\0';
        printf("Please enter the model: ");
        if (fgets(input[i].model, sizeof(input[i].model), stdin) == 0)
        {
            fprintf(stderr, "Unexpected EOF\n");
            break;
        }
        input[i].model[strcspn(input[i].model, "\n")] = '\0';
    }
    return i;
}

static void print_array(int num_entries, struct Guitars input[])
{
    for (int i = 0; i < num_entries; i++)
    {
        printf("%s %s\n", input[i].brand, input[i].model);
    }
}

int main(void)
{
    struct Guitars input[10];
    int n = input_data(10, input);
    print_array(n, input);
    return 0;
}

A sample run:

Please enter the brand: Gibson
Please enter the model: Les Paul
Please enter the brand: Fender
Please enter the model: Stratocaster
Please enter the brand: Unexpected EOF
Gibson Les Paul
Fender Stratocaster

You can easily argue that the 'Unexpected EOF' message is not good for the first (brand) input; it is more cogent for the second (model) input. It's easy to modify the code to suit your needs.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
  • If I may, it is a bit sloppy to use `fgets()` to read directly into the structure fields and remove the newline there. The fields must allow for this extra byte or it may be left pending in the input stream... A utility function with the appropriate behavior would be cleaner. – chqrlie Mar 03 '19 at 21:57
  • @chqrlie: yes, you're right — but it's Sunday and I was feeling lazy. In fact, I'd probably read into `char buffer[4096];` and then worry about whether there's space enough, etc. But that involves more error reporting, and I prefer to use my `stderr.c`/`stderr.h` code for that because it only needs one line per error instead of 3 or 4, but that complicates everything. So, I increased the field size to "big enough" to accept the existing data without any quibbles. I'd probably also arrange to remove leading and trailing white space, too, and regularize internal white space to a single blank. – Jonathan Leffler Mar 03 '19 at 22:01
  • Agreed. It is a pity the C Standard did not provide a decent replacement for `gets()`. – chqrlie Mar 03 '19 at 22:04
  • Yes, it is a pity. Annex K, function `gets_s()` is fairly close in that it provides the requisite interface, but the extra error checking makes it not quite suitable; if the line is too long, the whole line is thrown away (what was already read, and any remainder of the line). So, yes, there isn't a decent replacement. – Jonathan Leffler Mar 03 '19 at 22:09
  • One would want a `gets()` replacement with a stream, buffer, size and flags specifying what to do with the newline and how to handle the overlong lines. A global overflow handler is just about the worst solution for a real issue. – chqrlie Mar 03 '19 at 22:18