Switch statement and string copy

Hi all,

I'm having trouble with a function that is supposed to update records. I have copied it below:

Code:

---

void updaterecord(FILE *fptr)
{
        int record;
        int choice;
        char newname[30];
        char *nlptr;
        int newquantity;
        double newprice;

        struct inventory tool = {0, "", 0, 0.0};

        printf("Enter record to update(1-100): ");
        scanf("%d", &record);

        fseek(fptr, (record - 1) * sizeof(struct inventory), SEEK_SET);

        fread(&tool, sizeof(struct inventory), 1, fptr);

        if(tool.recordnum == 0){
                printf("Record has no information.\n");
        }/*end if*/
        else{
                printf("%-12d%-20s%-12d%12.2f\n",               
                        tool.recordnum, tool.toolname,
                        tool.quantity, tool.cost);

                printf("\nEnter 1 to update name.\n"
                                "2 to update quantity.\n"
                                "3 to update cost.\n"
                                "^Z to finish update.\nChoice? ");

                while((choice = getchar()) != EOF){                       

                        switch(choice){

                                case '1':
                                        printf("\n\nEnter new name: ");
                                        scanf("%s", newname);
                                        strcpy(tool.toolname, newname;
                                        if ((nlptr = strchr(newname, '\n')) != NULL){
                                                *nlptr = '\0';
                                        }
                                        break;

                                case '2':
                                        printf("\n\nEnter new quantity: ");
                                        scanf("%d", &newquantity);
                                        tool.quantity = newquantity;
                                        break;

                                case '3':
                                        printf("\n\nEnter increase (+) or reduction (-) of cost: ");
                                        scanf( "%lf", &newprice);
                                        tool.cost += newprice;
                                        break;

                                default:
                                        printf("Incorrect choice\n");
                                        break;
                        }/*end switch*/
                }/*end while*/

                printf("%-12d%-20s%-12d%12.2f\n",               
                        tool.recordnum, tool.toolname,
                        tool.quantity, tool.cost);

                fseek(fptr, (record - 1) * sizeof(struct inventory), SEEK_SET);

                fwrite(&tool, sizeof(struct inventory), 1, fptr);
        }/*end else*/
               
}

---

The trouble I have when I run the program is a little difficult for me to describe, so I copied a snapshot of it below:

Quote:

---

Press 1 to begin a new file, 2 to use existing data: 2

Enter your choice
1 - store a formatted text file of the inventory called
"storeinv.txt" for printing
2 - update a record
3 - add a new record
4 - delete a record
5 - end program
? 2
Enter record to update(1-100): 1
1 Buzz 7 12.99

Enter 1 to update name.
2 to update quantity.
3 to update cost.
^Z to finish update.
Choice? Incorrect choice
1


Enter new name: Buzz Saw
Incorrect choice
Incorrect choice
Incorrect choice
Incorrect choice
Incorrect choice
^Z
1 Buzz 7 12.99

Enter your choice
1 - store a formatted text file of the inventory called
"storeinv.txt" for printing
2 - update a record
3 - add a new record
4 - delete a record
5 - end program
?

---

My first question is, why does the program display "Incorrect choice" when I haven't, and second, how can I copy the new name properly? I've read the FAQ's about copying strings, and as far as I can tell, I have the right code. Thanks for the help.

> scanf("%s", newname);
This neither reads spaces, nor newlines

So in your example, " Saw\n" all get seen by the getchar() in your while loop as invalid input.

Don't mix input styles.
The most consistent approach is to always use fgets() for everything.

Say like this

Code:

---

    char buff[BUFSIZ];
    while( fgets(buff, sizeof buff, stdin) != NULL ){

      switch(buff[0]){

      case '1':
        printf("\n\nEnter new name: ");
        fgets(buff, sizeof buff, stdin);    // scanf("%s", newname);
        if ((nlptr = strchr(buff, '\n')) != NULL){  // do this before copying
          *nlptr = '\0';
        }
        strcpy(tool.toolname, newname);    // you were missing a ) here
        break;

      case '2':
        printf("\n\nEnter new quantity: ");
        fgets(buff, sizeof buff, stdin);
        sscanf(buff,"%d", &newquantity);
        tool.quantity = newquantity;
        break;
      default:
        printf("Incorrect choice\n");
        break;
      }/*end switch*/
    }/*end while*/

---

Thanks for the response. I used your suggestion of fgets, and unfortunately it's not working. Also, shouldn't there be parentheses after "sizeof"? Thanks again for the help.

Code:

---

while(fgets(choice, sizeof choice, stdin) != NULL){                       

                        switch(choice[0]){

                                case '1':
                                        printf("\nEnter new name: ");
                                        fgets(choice, sizeofchoice, stdin);
                                        if ((nlptr = strchr(newname, '\n')) != NULL){
                                                *nlptr = '\0';
                                        }
                                        strcpy(tool.toolname, fgets(newname,20,stdin));
                                        break;

                                case '2':
                                        printf("\nEnter new quantity: ");
                                        fgets(choice, sizeof choice, stdin);
                                        sscanf(choice,"%d", &newquantity);
                                        tool.quantity = newquantity;
                                        break;


                                case '3':
                                        printf("\nEnter new price: ");
                                        fgets(choice, sizeof choice, stdin);
                                        sscanf(choice,"%d", &newprice);
                                        tool.cost = newprice;
                                        break;

                                default:
                                        printf("Incorrect choice\n");
                                        break;
                        }/*end switch*/
                }/*end while*/

---

Quote:

---

Press 1 to begin a new file, 2 to use existing data: 2

Enter your choice
1 - store a formatted text file of the inventory called
"storeinv.txt" for printing
2 - update a record
3 - add a new record
4 - delete a record
5 - end program
? 2
Enter record to update(1-100): 1
1 Buzz 3 12.99

Enter 1 to update name.
2 to update quantity.
3 to update cost.
^Z to finish update.
Choice? Incorrect choice
1
Incorrect choice
1
Incorrect choice
2
Incorrect choice
3
Incorrect choice
^Z
1 Buzz 3 12.99

Enter your choice
1 - store a formatted text file of the inventory called
"storeinv.txt" for printing
2 - update a record
3 - add a new record
4 - delete a record
5 - end program
? 5
Press any key to continue . . .

---

> strcpy(tool.toolname, fgets(newname,20,stdin));
Just choice, it's what you read into earlier

> fgets(choice, sizeofchoice, stdin);
There's something wrong with your copy/paste, this doesn't compile.

>> My first question is, why does the program display "Incorrect choice" when I haven't

You probably need to clear the newline from the input buffer that scanf left behind.

>> how can I copy the new name properly?

It look like you are (except in the code you posted there's a missing paren - typo?). You're removing the newline from the source string *after* you made a copy of it, though. And at any rate, you don't even need the temporary buffer - just pass the structure member directly to scanf.

One more thing I want to reiterate is that it is imperative that you check your return values. If the user enters an invalid number, for instance, your application won't even notice and just stuffs a garbage value into the structure. The rule of thumb is any information that you have no direct control over (eg: user input) should be rigorously validated before it is accepted into the program. Otherwise, you're just opening yourself up to unnecessary bugs. This included paying attention to input lengths as well. For instance, let's say the user enters 50 characters for the product name. If you don't tell scanf what it's limit is, this is going to cause a buffer overflow. When that happens, your program will act erratically and it may be hard to pinpoint the bug (it might not even surface immediately). Example:

Code:

---

int main( void )
{
        int
                integer = 1024;
        char
                buffer[ 1 ];
        printf( "Enter some text > " );       
        if( scanf( "%s", buffer ) != 1 )
                puts( "Error: invalid input" );
        else if( integer != 1024 )
                printf( "That's wierd, integer equals %d (should be 1024)\n", integer );
        return 0;               
}

---

Obviously, the variable 'integer' was overwritten by the input - not a good situation! So in this trivial case, the format string should have been "%0s" since the only buffer consists of a single byte (and needs to leave room for the null terminator). So in other words, the format string should specify a value equal to N-1, where N is the size of the buffer.


EDIT:
Damn, I really was to slow to post. I'll need to catch up. :p

Thanks for the help! I was trying to experiment to see if I could read the input and automatically copy the string. So I have it working, but the buffer still has some characters(not sure how). See below:

Quote:

---

Press 1 to begin a new file, 2 to use existing data: 2

Enter your choice
1 - store a formatted text file of the inventory called
"storeinv.txt" for printing
2 - update a record
3 - add a new record
4 - delete a record
5 - end program
? 2
Enter record to update(1-100): 1
1 1 3 12.99

Enter 1 to update name.
2 to update quantity.
3 to update cost.
^Z to finish update.
Choice? 1
Choice?
Enter new name: Buzz Saw

Choice? ^Z
1 Buzz Saw 3 12.99

Enter your choice
1 - store a formatted text file of the inventory called
"storeinv.txt" for printing
2 - update a record
3 - add a new record
4 - delete a record
5 - end program
? 5
Press any key to continue . . .

---

The update part of the program still works, but you have to input a newline character each time you update a part of the record. For instance, after I typed in "Buzz Saw" to change the name and hit enter, I had to hit enter again for "Choice?" to print.

>> The update part of the program still works, but you have to input a newline character each time you update a part of the record.

It's difficult to say without seeing the updated code. ;)