views:

1011

answers:

3

I'm writing a program that works with files. I need to be able to input data as structures, and eventually read it out. The problem i have at the moment is with this code:

typedef struct {
    char* name;
    .....
}employeeRecord;
employeeRecord record;

char name[50];

if(choice == 1)
    {
        /*Name*/
        printf("\nEnter the name:");
        fgets(name,50,stdin);
        record.nameLength = strlen(name) -1;
        record.name = malloc(sizeof(char)*record.nameLength);
        strcpy(record.name,name);
        /*Other data, similar format...*/

If i want for example, name address and phone number, and ask for each in a row (so address is pretty much identical to above except replacing 'name' with address), i find it skips the input. What i mean is, I am given no chance to input it. The output is actually Enter the name: Enter the address: (and here is where it prompts me for input)

A: 

I tried your code and can't reproduce the problem. The following code works just the way you would expect, it prompts for the name, wait for you to type the name, then prompts for the address, etc.

I'm wondering if you don't need to read stdin and empty it before you prompt for more input?

typedef struct {
    char* name;
    char* address;
}employeeRecord;

int readrecord(employeeRecord &record)
{
   char name[50];
   char address[100];

   printf("\nenter the name:");
   fgets(name, sizeof(name), stdin);
   record.nameLength = strlen(name) + 1;
   record.name = malloc(sizeof(char)*record.nameLength);
   strcpy(record.name,name);

   printf("\nenter the address:");
   fgets(address, sizeof(address), stdin);

   ...    
}

Incidently, you want to add 1 to strlen(name), not subtract 1. or, if you want name stored in your record without a terminating null, then you need to use memcpy to copy the string into your record, not strcpy.

Edit:

I see from comments that you are using scanf to read the choice value, this is leaving a \n in the input buffer which is then picked up by your first fgets call. What you should do instead is to use fgets to read in the choice line, and then sscanf to parse the value out of the input. like this

int choice;
char temp[50];
fgets(temp, sizeof(temp), stdin);
sscanf(temp, "%d", &choice);

that should make the whole issue of flushing stdin moot.

John Knoeller
There must be a zillion comments on StackOverflow already about how fflush(stdin) isn't a supported anything, and if it works, it is only by accident.
Zan Lynx
@Zan Lynx: by the way, it's not an accident fflush(stdin) works on Windows, they did it on purpose. It's only *nix platforms that think having fflush be undefined for stdin is a good idea.
John Knoeller
@John Knoeller: thanks a bunch for your edit, thats exactly what i was looking for. And it works perfect now, much appreciated.
Blackbinary
@John: Wrong, the standard explicitly states "If stream points to an output stream or an update stream in which the most recent operation was not input, the fflush function causes any unwritten data for that stream to be delivered to the host environment to be written to the file; otherwise, the behavior is undefined."This isn't "*nix" platform implementation defined
Mustapha Isyaku-Rabiu
@rogue: no you are wrong. http://msdn.microsoft.com/en-us/library/9yky46tz(VS.71).aspx clearly states that fflush(stdin) works with MSVC, and is in fact _used in their example_. Its is only *nix platforms that do the wrong thing here.
John Knoeller
@John: Yes, as an extension.
Mustapha Isyaku-Rabiu
@John: It's not the 'wrong' thing, the C standard leaves the behaviour as 'undefined'. Even your link says as much: '// fflush on input stream is an extension to the C standard'. Now, you may say that that is the behaviour it should be, but the standard doesn't define anything (and as a fsync() syscall to the stdin descriptor on a \*nix system makes no sense ('sync pending writes to stdin to disk'), fflush on the FILE\* stdin similarly makes little sense *in that environment*. And as it's unportable, one should avoid using it if possible (just like when using GNU extensions).
Bernard
Whoah. Apologies for the novel. ^^;;
Bernard
@Barnard: Its wrong because it's unreasonable for it to be undefined behavior. It makes no sense. The compiler doesn't complain when you fflush(stdin), the types are compatible and the intent is entirely clear. The standard is just broken.
John Knoeller
I read flush as meaning 'flush to destination'. As stdin is to be read from, doesn't make any sense to me. Now I'm a Unix programmer, and as I said before, semantically I read fflush as fsync for FILE pointers. I take from your profile that you are primarily a Windows programmer. As you use a different environment, your expectations of fflush are different. So Windows does what you'd expect. But it's not like that everywhere, because the users of different environments have different expectations. Seeing as C and Unix grew up together, you can see why the C standard leaves it as undefined.
Bernard
@Bernard: Fair enough. It's useful to know that it's not portable.
John Knoeller
And, it appears there is a separate function for this purpose: http://linux.about.com/library/cmd/blcmdl3_fpurge.htm Unfortunately, it seems unportable to MSVC! You just can't win. =p (of course, seeing as on MSVC fflush does the same thing, a simple wrapper function is, well, simple!)
Bernard
http://c-faq.com/stdio/stdinflush.html
jamesdlin
+3  A: 

The newline is still in stdin from a prior call to a function that didn't read a newline from input. Clear stdin by reading until you've read out the newline -- not by flushing stdin as others have suggested.

EDIT: Thanks Alok, for the correction!

Bernard
Like Bernard says, one should be careful to never leave trailing whitespace unread (like scanf does), if line-oriented input is intended. Reading everything a line at a time (e.g. via fgets) and then using sscanf on the line read in one suitable option.
Tronic
If `fgets` read the whole line, the newline character is *not* in the `stdin` buffer. Most likely the OP is reading `choice` by one of `getchar()`, `getc()`, `fgetc()` or equivalent. If `fgets` didn't read the whole line, the input buffer needs to be bigger. (Even the quote in your answer says that the newline character is read.)
Alok
Alok: you are correct, brainfart. :o
Bernard
By means of: "while (getc(stdin) != '\n');" -- fflush(stdin) should not be used for it is undefined behaviour.
Mustapha Isyaku-Rabiu
I am getting choice as a simple int, through scanf("%d",I have semi-solved it by repeating the fgets calls. So im still unclear about what to do...
Blackbinary
+2  A: 

You probably used scanf to read choice before calling fgets to read the name. scanf may have left a newline in stdin which your code mistakes for an empty name input. If that is indeed the case, try not to use scanf (use fgets to retrieve choice and use atoi to conver to int or strcmp to compare against "1\n" etc.). The code should otherwise work, with the below modifications to account for the fact that fgets also reads the terminating newline into the buffer (which you probably want stripped):

  #define MY_LENOF(x) (sizeof(x)/sizeof((x)[0])) 

  char choice[3] = { 0 }; /* example of how to initialize to all NULs */
  if (!fgets(choice, MY_LENOF(choice), stdin)) {
    fprintf(stderr, "Premature end of input\n");
    exit(1);
  }

  if (strcmp(choice, "1\n") == 0) {  
    /*Name*/
    printf("\nEnter the name:");
    if (!fgets(name, MY_LENOF(name), stdin)) {
      /* if fgets fails it leaves name unchanged, so we reset it to "" */
      name[0] = '\0';
    }
    /* good practice to use srtnlen in order not to overrun fixed buffer */
    /*  not necessarily a problem with fgets which guarantees the trailing NUL */
    size_t nameLength = strnlen(name, MY_LENOF(name));
    assert(name[nameLength] == '\0');
    if (nameLength - 1 > 0 && name[nameLength - 1] == '\n') {
      /* strip trailing newline */
      name[--nameLength] = '\0';
    } else if (nameLength >= MY_LENOF(name) - 1) {
      fprintf(stderr, "Name is too long\n");
      exit(1);
    } else {
      fprintf(stderr, "Premature end of input\n");
      exit(1);
    }

    record.nameLength = nameLength;
    record.name = malloc(sizeof(char)*(record.nameLength + 1));
    strcpy(record.name, name);
vladr
Probably don't want to use `atoi`, as it performs no error checking (`sscanf` and check the return value should work, my C is rusty though). Nice answer.
Bernard
correct. therefore, in his case (I imagine he performs no arithmetic, just checking) he should use `strcmp`
vladr
@Bernard: `sscanf` is pretty crummy for error-checking too if you're not careful (e.g. `"%d"` will match `"123foo"`). Checking for malformed input with `strtod` and the like is much more straightforward.
jamesdlin
Like I said, my C is rusty! =D
Bernard