The first char of my output is omitted in C

Question

I can't seem to figure out what is going on with my output. I am reading in multiple lines of user input and outputting corresponding input that exceeds a lower boundary. For some reason when I output, the string that's outputted is omitting the first character of the string. Can anyone tell me why this is occuring?

#include <stdio.h>

typedef struct{
   char  name[4];
   int   population;
} state;

enum { MAX_STATES = 10 };

int main()
{
    state myStates[MAX_STATES];

    int c;
    int i = 0;

    while ((c = getchar())!= EOF)
    {
        scanf("%s %d\n", myStates[i].name, &myStates[i].population);
        i++; 
    }

    // printf("Last character is [%d]\n", c);
    printf("");

    if (c <= 0)
    {
        for(int j = 0; j <= MAX_STATES; j++)
        {
            if(myStates[j].population >= 10)
                printf("%s %d\n", myStates[j].name, myStates[j].population);
            else
                break;
        }
    }

    return 0;
}

Input:

TX 23
CA 45

Output:

X 23
A 45

Updated Code:

#include <stdio.h>

typedef struct{
   char  name[4];
   int   population;
} State;

enum { MAX_STATES = 10 };

int main()
{
    State myStates[MAX_STATES];

    int i, j;

    // Function to read in multiple lines (up to 10) of user input; loop 
    // controls in place, detects format problems, prevents string buffer 
    // overflows.
    for (i = 0; i < MAX_STATES; i++)
    {
        if (scanf("%2s %d\n", myStates[i].name, &myStates[i].population) != 2)
            break;
    }

    // Function to output (stdout) array of State structs that exceed 10 
    // population. 
    for(j = 0; j < i; j++)
        {
            if(myStates[j].population >= 10)
                printf("%s %d\n", myStates[j].name, myStates[j].population);
            else
                break;
        }

    return 0;
}

The output as posted, only goes until there is an input that is less than 10 and it breaks out of the loop. When I didn't have that break statement, I was getting garbage output at the last line. Any suggestions to improve the output?

Probably because it's getting read in your `getchar` call before you assign it to the state name. Not knowing what your input is exactly, I would check that first and see if it's your output or your input function. — Kdawg, Jul 19 '17 at 03:36
I edited to reflect the input separate from the output. If the getchar() is the problem, how do I replace it meanwhile keeping the while() loop? — Nathan1324, Jul 19 '17 at 03:47

Jonathan Leffler · Accepted Answer · 2017-07-19T13:38:53.593

Replace:

int i = 0;

while ((c = getchar()) != EOF)
{
    scanf("%s %d\n", myStates[i].name, &myStates[i].population);
    i++; 
}

with:

int i;

for (i = 0; i < MAX_STATES; i++)
{
    if (scanf("%3s %d", myStates[i].name, &myStates[i].population) != 2)
        break;
}

This protects you against entering too many states, uses the for loop to put the loop controls in place, detects format problems, prevents string buffer overflows, and reads the first character into the name. Also, a trailing white space (such as a blank or newline) in a format string is a very bad idea in a scanf() format string if the input is being entered interactively. If the input comes from a file, it is less serious but still unnecessary most of the time. (See Trailing blank in scanf() format for more information.)

Keeping a while loop

If you're really adamant that you need a while loop, then you can use:

int i = 0;

while (i < MAX_STATES && (c = getchar()) != EOF)
{
    ungetc(c, stdin);
    if (scanf("%3s %d", myStates[i].name, &myStates[i].population) != 2)
        break;
    i++; 
}

or:

int i = 0;

while (i < MAX_STATES && (c = getchar()) != EOF)
{
    myStates[i].name[0] = c;
    if (scanf("%2s %d", &myStates[i].name[1], &myStates[i].population) != 2)
        break;
    i++; 
}

Note that these while loops still maintain both lots of overflow protection — overflowing the main array, and overflowing the name field. Note that one of the two scanf() statements uses %3s and the other %2s; you should be able to explain why. (And yes, the null byte is not counted by scanf(), so you have to use an 'off-by-one' length in the conversion specification.)

There are, no doubt, other techniques that could also be used. However, I think you'll find that the for loop is more nearly idiomatic C.

One alternative that is often sensible is to use fgets() (or POSIX getline() if it is available) to read whole lines, and then sscanf() to parse the lines. This often leads to more resilient programs, and better error reporting. It also stops people who try to put the information for all 50 states on a single line, or who put each datum on a separate line with a blank line in between them all, from getting away with the malformed data. You can quietly insist on two fields (and, if you're careful, only two fields) on the line.

And the output code?

May I inquire about a suggestion for displaying the output properly?

You have:

printf("");

if (c <= 0)
{
    for(int j = 0; j <= MAX_STATES; j++)
    {
        if(myStates[j].population >= 10)
            printf("%s %d\n", myStates[j].name, myStates[j].population);
        else
            break;
    }
}

The first printf() does nothing; it should go. The if (c <= 0) condition is a bit dubious. It is possible to type a null byte (often Control-@ or Control-Shift-2), though it would be a bit hard to get that to break the original loop. The for loop should be more like for (int j = 0; j < MAX_STATES; j++) — this is the template for safe for loops in C. You most frequently use for (int i = 0; i < MAX; i++). However, you only want to print the states that were read, so instead of using MAX_STATES, you need to use i as the limit. If you really only want to print the top 9 states (CA, TX, FL, NY, IL, PA, OH, GA, NC — see Wikipedia; Michigan is just shy of 10M, it says), then the if condition is fine.

So, you could use (noting that the input loop sets i to the number of states read successfully):

for (int j = 0; j < i; j++)
    printf("State: %.2s, Pop'n: %dM\n", myStates[j].name, myStates[j].population);

You can tweak the format to suit your requirements, of course. This will print nothing if no states were read, or the number of states that were read. If you really want to apply the condition on the population, then you'd use:

for (int j = 0; j < i; j++)
{
    if (myStates[i].population >= 10)
        printf("State: %.2s, Pop'n: %dM\n", myStates[j].name, myStates[j].population);
}

Sorry, I edited the original post to reflect 'scanf()' rather than fscanf(). Other than that, I would need to use the while() loop because I am reading in multiple lines of user inputted data from stdin, which ends when the EOF flag is entered (ctrl-d). Then, the output is spit out to stdout. — Nathan1324, Jul 19 '17 at 03:46
This code handles EOF (by testing the return value from `scanf()` — now I've fixed that), and also prevents overflow by not loading more rows than will fit in the array. The use of `%3s` ensures that if someone writes `Texas` instead of `TX`, then things will not go haywire (but it will allow `Cal` or `CA` for California). — Jonathan Leffler, Jul 19 '17 at 03:48
Thanks so much! So I had to readjust my output to respond correctly to the scanf(). The output looks much better now! — Nathan1324, Jul 19 '17 at 04:22
May I inquire about a suggestion for displaying the output properly? I tried an if-else inside a similar for-loop and it works until I reach a condition that doesn't meet the 10 lower limit and it breaks out of the loop. If I don't have the else --> break, I am getting a crazy output of garbage at the end. — Nathan1324, Jul 19 '17 at 04:41
For educational purposes, the population numbers are arbitrary. I tried the above for-loop and my output came out: TX 23 DE 3 CA 25 TX 23 CA 25 ??MR? 32767 Any idea what that last line is like that? It's pretty consistent with just the string portion altered with each trial. — Nathan1324, Jul 19 '17 at 04:59
Nevermind! You answered it previously... the solution was having the condition (j < i) rather than MAX_STATES. Thanks again for your help! — Nathan1324, Jul 19 '17 at 05:01
In the updated code, you have `for(j = 0; j <= MAX_STATES; j++)` controlling the printing. That accesses the `myStates[10]` element of the array, which doesn't actually exist (the array size is 10, so the maximum valid index is 9) — you're getting undefined behaviour because you're abusing the array size. It so happens that what it is treating as the population is bigger than 10, so it prints the junk. — Jonathan Leffler, Jul 19 '17 at 05:02
but `scanf` return value can be != 2 for other reasons, including malformed input... — Antti Haapala -- Слава Україні, Jul 19 '17 at 05:47
There are multiple ways to deal with it — I choose to stop the input on any format error or EOF. You may write your own answer if you wish to do other things, @AnttiHaapala. — Jonathan Leffler, Jul 19 '17 at 05:49
Maybe this is a leftover from the earlier `fprintf()` version, but shouldn't you remove the trailing `\n` from the `scanf()` format string? As it is, the user has to signal end-of-file twice from the keyboard to terminate the input loop, and this trailing whitespace character serves no purpose that I can see. (Of course, entering a character in the second field works as expected for loop-termination). — ad absurdum, Jul 19 '17 at 08:10
@DavidBowling: thanks. The trailing newlines in the `scanf()` format are a bad idea. I've removed them and commented on doing so. I'll add a link to the canonical Q&A later. (And thanks for the other edit.) — Jonathan Leffler, Jul 19 '17 at 12:44

Anoop · Answer 2 · 2017-07-19T06:06:23.403

0

You could try adding a space before %s in scanf, or specify a strict number of chars.

scanf(" %3s",

Or even use as in Beej's guide:

// read all whitespace, then store all characters up to a newline 

scanf(" %[^\n]", s);

edited Jul 19 '17 at 06:06

answered Jul 19 '17 at 04:11

Anoop

1
2

Note that the name is `char name[4];`, so it is crucial to use `%3s` in the `scanf()` conversion specification — `scanf()` doesn't count the terminating null, but does add one. – Jonathan Leffler Jul 19 '17 at 04:27

score 0 · Answer 3 · answered Jul 19 '17 at 04:26

0

An alternative is:

int i = 0;
char temp[100];
for(i=0; i<MAX_STATES; i++){
    fgets(temp, 100, stdin);
    if(strcmp(temp, "\n") == 0)
        break;
    sscanf(temp, "%s %d\n", myStates[i].name, &myStates[i].population);
}

answered Jul 19 '17 at 04:26

VIX

605
4
15

You need to test the return value from `sscanf()`. Otherwise good, though. – Jonathan Leffler Jul 19 '17 at 04:42

score 0 · Answer 4 · answered Jul 19 '17 at 06:07

0

You could try adding a space before %s in scanf, or specify a strict number of chars.

Or even use this:

// read all whitespace, then store all characters up to a newline

scanf(" %[^\n]", s);

answered Jul 19 '17 at 06:07

Anoop

1
2

There is no need to add a leading whitespace character before an `%s` directive with `scanf()`, since this directive automatically skips over leading whitespace. It does make sense to add this before `%[]` directives, which do not ignore leading whitespace characters. Still, this does not solve OP's problem, which is the `getchar()` in this line: `while ((c = getchar())!= EOF) {}`. – ad absurdum Jul 19 '17 at 12:11

The first char of my output is omitted in C

4 Answers4

And the output code?