fgets() - supposed to be simple :)

Discussion in 'C Programming' started by Rob Somers, Nov 23, 2003.

  1. Rob Somers

    Rob Somers Guest

    Hey all

    I am writing a program to keep track of expenses and so on - it is not
    a school project, I am learning C as a hobby - At any rate, I am new
    to structs and reading and writing to files, two aspects which I want
    to incorporate into my program eventually. That aside, my most
    pressing problem right now is how to get rid of the newline in the
    input when I use fgets(). Now I have looked around on the net, not so
    much in this group as on another C board, and I have found several
    good solutions for getting rid of the newline character, however I
    cannot seem to make it work for me, and now my program is becoming
    more unmanageable as I go along, trying to fix the problem. I will
    post the code, and I will comment in the part that I am specifically
    not understanding.

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

    #define CLEAR printf("\033[2J\033[0;0f");
    #define MAX 100

    /* structure for writing to and reading from file */

    struct money{
    char category[MAX];
    char month[MAX];
    int day[MAX];
    char purchase_description[MAX];
    float purchase[MAX];
    };

    void menu( void );
    int get_num( void );
    int validate( char *a );
    void add_record( void );
    int main( void )
    {
    menu();
    return 0;
    }

    /* menu() - display a menu and return the choice of the user */
    void menu( void )
    {
    char choice;
    printf( "\t\t\tMoney Log \n\n\n" );
    printf( "\t1. Update your record\n\n" );
    printf( "\tPlease enter your selection: " );

    choice = get_num();
    switch( choice ){
    case 1:
    CLEAR;
    add_record();
    break;
    default:
    break;
    }

    }

    /* add_record - Read from keyboard and add the record in the list */
    void add_record( void )
    {
    char buffer[MAX];
    char *p = NULL;

    struct money money_log;


    /* This next bit is supposed to ask the user for each piece of
    information,and store the input in the struct - It is where my problem
    lies I think */

    printf ( "Enter category of purchase: " );
    fgets( buffer, MAX, stdin );
    if( ( p = strchr( buffer, '\n' ) != NULL )
    *p = '\0';
    strcpy( buffer, money_log.category );
    fflush( stdout );


    printf( "Enter month (eg, Oct.): " );
    fgets( buffer, MAX, stdin );
    if( ( p = strchr( buffer, '\n' ) != NULL )
    *p = '\0';
    strcpy( buffer, money_log.month );
    fflush( stdout );

    printf( "Enter day: " );
    fgets( buffer, MAX, stdin );
    money_log.day = atoi( buffer );
    if( ( p = strchr( buffer, '\n' ) != NULL )
    *p = '\0';
    fflush( stdout );

    /* I have been experimenting here (above) with different things, and I
    get a pile of warnings on my compiler which I will post also. */



    printf ("Enter a description of the purchase (eg, phone bill):
    ");
    fgets(money_log.purchase_description, MAX, stdin);

    printf ("Enter the amount of the purchase : ");
    fgets(buffer, MAX, stdin);
    money_log.purchase = atoi( buffer );

    CLEAR;
    printf( "Category\tDate\tPurchase\tAmount\n\n" );
    printf( "%s %s%d %s %.2f", money_log.category,
    money_log.month, money_log.day, money_log.purchase_description,
    money_log.purchase );
    }

    /* get_num() - get a number from the user, and check it for validity
    */
    int get_num( void )
    {
    int choice;
    char buffer[BUFSIZ];

    if( fgets ( buffer, sizeof buffer, stdin ) != NULL ){
    buffer[strlen ( buffer ) - 1] = '\0';
    if( validate( buffer ) == 0 ){
    choice = atoi( buffer );
    }
    else
    printf( "\nPlease enter a numerical character." );
    }
    else
    printf( "Error reading input\n" );

    return choice;
    }

    /* validate() - connected to get_num() */
    int validate( char *a )
    {
    unsigned x;
    for ( x = 0; x < strlen ( a ); x++ )
    if( !isdigit ( a[x] ) ) return 1;

    return 0;
    }

    Here are warnings gcc gives me when I try to compile:

    another.c: In function `add_record':
    another.c:62: warning: assignment makes pointer from integer without a
    cast
    another.c:63: invalid operands to binary *
    another.c:63: parse error before ';' token
    another.c:70: warning: assignment makes pointer from integer without a
    cast
    another.c:71: invalid operands to binary *
    another.c:71: parse error before ';' token
    another.c:77: incompatible types in assignment
    another.c:78: warning: assignment makes pointer from integer without a
    cast
    another.c:79: invalid operands to binary *
    another.c:79: parse error before ';' token
    another.c:88: incompatible types in assignment
    another.c:92: warning: int format, pointer arg (arg 4)
    another.c:92: warning: double format, pointer arg (arg 6)

    The reason the newline is bothering me is that I would like the data
    to print out in one line, instead of many. At any rate, if any of you
    could set me straight on getting rid of the newline with fgets() in
    this particular case, I would be grateful.

    Rob Somers
    BTW, I am not *as* worried about this program being portable (hence
    the #define CLEAR statement ) as it is only meant to run on my own
    machine here at home.
     
    Rob Somers, Nov 23, 2003
    #1
    1. Advertising

  2. Rob Somers wrote:

    >
    > #include <stdio.h>
    > #include <stdlib.h>
    > #include <string.h>
    > #include <ctype.h>
    >
    > #define CLEAR printf("\033[2J\033[0;0f");


    Bad way to define a macro. What happens when I try to do this:

    if (some_condition)
    CLEAR;
    else
    some_func();

    ?

    <snip>

    >
    > /* add_record - Read from keyboard and add the record in the list */
    > void add_record( void )
    > {
    > char buffer[MAX];
    > char *p = NULL;
    >
    > struct money money_log;
    >
    >
    > /* This next bit is supposed to ask the user for each piece of
    > information,and store the input in the struct - It is where my problem
    > lies I think */
    >
    > printf ( "Enter category of purchase: " );
    > fgets( buffer, MAX, stdin );
    > if( ( p = strchr( buffer, '\n' ) != NULL )


    You are missing a parentheses.

    > *p = '\0';
    > strcpy( buffer, money_log.category );
    > fflush( stdout );


    Your indenting suggests that you expect the strcpy and fflush to be
    controlled by the 'if'. They are not. The control block for the 'if' is
    only the next statement (*p = '\0';). If you want more than one
    statement, you have to group them inside brackets:

    if( ( p = strchr( buffer, '\n' ) != NULL )
    {
    *p = '\0';
    strcpy( buffer, money_log.category );
    fflush( stdout );
    }

    I don't see anything wrong with the method you are using to remove the
    newline. The errors you are getting seem to be related to other
    problems. If the program is too large for you to debug, create a smaller
    program to test your fgets usage.

    -Kevin
    --
    My email address is valid, but changes periodically.
    To contact me please use the address from a recent posting.
     
    Kevin Goodsell, Nov 23, 2003
    #2
    1. Advertising

  3. Rob Somers

    Kurt Watzka Guest

    Rob Somers wrote:


    > printf ( "Enter category of purchase: " );
    > fgets( buffer, MAX, stdin );
    > if( ( p = strchr( buffer, '\n' ) != NULL )


    This is a syntax error.

    You probably want

    if ( (p = strchr(buffer, '\n') ) != NULL )

    > *p = '\0';
    > strcpy( buffer, money_log.category );
    > fflush( stdout );
    >


    Please look up strcpy in your documentation to find out which
    argument is the source string and which argument is the target
    string.

    Kurt
     
    Kurt Watzka, Nov 23, 2003
    #3
  4. Rob Somers

    Rob Somers Guest

    Kurt Watzka <-muenchen.de> wrote in message news:<bpreqp$pf1$00$-online.com>...
    > Rob Somers wrote:
    >
    >
    > > printf ( "Enter category of purchase: " );
    > > fgets( buffer, MAX, stdin );
    > > if( ( p = strchr( buffer, '\n' ) != NULL )

    >
    > This is a syntax error.
    >
    > You probably want
    >
    > if ( (p = strchr(buffer, '\n') ) != NULL )
    >
    > > *p = '\0';
    > > strcpy( buffer, money_log.category );
    > > fflush( stdout );
    > >

    >
    > Please look up strcpy in your documentation to find out which
    > argument is the source string and which argument is the target
    > string.
    >
    > Kurt


    Both of your replies were very helpful to me. Thank you for taking
    the time to respond.

    Rob
     
    Rob Somers, Nov 24, 2003
    #4
  5. Rob Somers

    Malcolm Guest

    "Rob Somers" <> wrote in message
    > The reason the newline is bothering me is that I would like the data
    > to print out in one line, instead of many. At any rate, if any of you
    > could set me straight on getting rid of the newline with fgets() in
    > this particular case, I would be grateful.
    >

    fgets() is a nuisance.

    The idea is that
    char buff[100];

    gets(buff);

    will cause a buffer overrun and undefined behaviour if the user types in
    more than 100 characters.

    so fgets(buff, 100, stdin);

    solves the problem by only reading in a maximum of 99 characters.

    However what happens when the user enters over 100 characters? The answer is
    that the line is half-read. How do you know it is a partial read? Because
    there is no newline at the end.

    Use strchr(buff, '\n') to detect the newline. If it is there, remove it. If
    it is not there, and this is the difficult part, you have a partial read and
    you have to read the garbage (eg fgetc() up to the newline or EOF). Then you
    have to output some sort of error message to the user.
     
    Malcolm, Nov 24, 2003
    #5
  6. Groovy hepcat Rob Somers was jivin' on 23 Nov 2003 14:08:26 -0800 in
    comp.lang.c.
    fgets() - supposed to be simple :)'s a cool scene! Dig it!

    >I am writing a program to keep track of expenses and so on - it is not
    >a school project, I am learning C as a hobby - At any rate, I am new
    >to structs and reading and writing to files, two aspects which I want
    >to incorporate into my program eventually. That aside, my most
    >pressing problem right now is how to get rid of the newline in the
    >input when I use fgets(). Now I have looked around on the net, not so


    You apparently didn't look very hard, then.

    >much in this group as on another C board, and I have found several


    Why didn't you read this newsgroup first? It is very rude to post to
    a newsgroup without first reading at least a month worth of articles
    therein. This has been standard practice since the early days of the
    Internet, and has been ignored by the scores of newbies who have
    posted here since the Net became mainstream.
    Why didn't you read this newsgroup's FAQ either? It is extremely
    rude to post to a newsgroup without first reading its FAQ list. This
    has also been standard practice for a long time, but now, sadly,
    ignored by many. Find the FAQ at this URL:

    http://www.eskimo.com/~scs/C-faq/top.html

    >good solutions for getting rid of the newline character, however I
    >cannot seem to make it work for me, and now my program is becoming
    >more unmanageable as I go along, trying to fix the problem. I will
    >post the code, and I will comment in the part that I am specifically
    >not understanding.


    OMG! You've posted your entire program! That is a big no no. You
    should have cut it down to the smallest complete program that
    demonstrates the problem. By "the smallest complete program" I mean
    something that (if corrected) will compile and run, but only does
    enough to show what the problem is. We don't want to have to wade
    through an entire program just to see what's wrong with one small part
    of it. You should do the work (of cutting it down), not us. After all,
    you're the one who wants our help.
    However, there are many problems throughout your code that I have
    addressed below.
    Another thing: you really need to work on your code formatting. Your
    code is rather ugly visually, due to a total lack of consistency in
    indenting and use of white space. This makes your code harder to read
    and understand. Please adopt a legible white space and indentation
    style, and stick to it. (Use spaces, not tabs.) Where several lines
    perform a simple operation together, group these lines together and
    surround them with white space (newlines). If it is not immediately
    obvious what these groups of lines do, then include a brief, concise
    comment that describes, in plain language, what the intention is. This
    will greatly aid in understanding the logic of your program.

    >#include <stdio.h>
    >#include <stdlib.h>
    >#include <string.h>
    >#include <ctype.h>
    >
    >#define CLEAR printf("\033[2J\033[0;0f");


    Poor use of a macro. You're using an object-like macro to call a
    function, which is counter intuitive. You're also ending the
    replacement text with a semicolon, which means you don't need to (and,
    in some cases, shouldn't) use the macro with a semicolon. It's not
    even a portable way of clearing the screen (if that's what it's meant
    to do). In fact, there is no portable way to do so. There are C
    implementations that run on systems that don't even have screens.
    But, as you said, you're not overly concerned about portability.
    However, we are. You should have cut out everything non-portable
    before posting.

    >#define MAX 100
    >
    >/* structure for writing to and reading from file */
    >
    >struct money{
    > char category[MAX];
    > char month[MAX];
    > int day[MAX];


    You need an array of 100 ints to store a day?

    > char purchase_description[MAX];
    > float purchase[MAX];


    And you need an array of 100 floats for the amount of the purchase?
    BTW, double is usually a better choice, where a floating type is
    needed. It is the "natural" floating type in C, and typically has
    higher precision than float.
    But actually, the best type for the purchase price is not a floating
    type at all. An integer type (representing, for example, the number of
    cents instead of dollars) would be better. Due to the way floating
    types are represented, they are not accurate enough for something like
    this.

    >};
    >
    >void menu( void );
    >int get_num( void );
    >int validate( char *a );
    >void add_record( void );
    >int main( void )
    >{
    > menu();
    > return 0;
    >}


    Poor design. main() becomes nothing more than a wrapper, which makes
    it rather useless and just adds to the function call overhead (not
    that that matters much in a program of this nature).

    >/* menu() - display a menu and return the choice of the user */
    >void menu( void )


    You should probably do some kind of error testing, and return an
    error indicator to main(). main() would then handle the error in some
    way. Then main() would be more than just a wrapper. It would have some
    purpose after all.

    >{
    > char choice;


    Why char?

    > printf( "\t\t\tMoney Log \n\n\n" );
    > printf( "\t1. Update your record\n\n" );
    > printf( "\tPlease enter your selection: " );


    You need to flush the output here to make sure the prompt is
    displayed before the program waits for input.

    fflush(stdout);

    > choice = get_num();
    > switch( choice ){
    > case 1:
    > CLEAR;

    ^
    Ande here you've forgotten that you've ended this macro's
    replacement text with a semicolon (see above), so you have an extra
    semicolon here. IOW, this line expands to this:

    printf("\033[2J\033[0;0f");;
    ^
    Note the extra semicolon. This is not an error (in this case), but is
    not really wanted.

    > add_record();


    add_record() should probably return an error indicator, which could
    perhaps be returned to main(), or could be intercepted here.

    > break;
    > default:
    > break;
    > }
    >}
    >
    >/* add_record - Read from keyboard and add the record in the list */
    >void add_record( void )
    >{
    > char buffer[MAX];
    > char *p = NULL;
    >
    > struct money money_log;
    >
    >/* This next bit is supposed to ask the user for each piece of
    >information,and store the input in the struct - It is where my problem
    >lies I think */
    >
    > printf ( "Enter category of purchase: " );


    Again you need to flush the output.

    fflush(stdout);

    > fgets( buffer, MAX, stdin );
    > if( ( p = strchr( buffer, '\n' ) != NULL )

    ^
    You have a missing closing parenthesis here. That is the likely
    casue of your problem.

    > *p = '\0';
    > strcpy( buffer, money_log.category );


    Your arguments are the wrong way around.

    > fflush( stdout );


    Why on Earth are you flushing stdout here? You haven't written
    anithing to stdout here. A few lines above, yes; the line below, yes;
    but not here.

    > printf( "Enter month (eg, Oct.): " );


    fflush(stdout);

    > fgets( buffer, MAX, stdin );
    > if( ( p = strchr( buffer, '\n' ) != NULL )

    ^
    And once again you have a missing parenthesis.

    > *p = '\0';
    > strcpy( buffer, money_log.month );


    Wrong way around.

    > fflush( stdout );


    And again, what is the purpose of flushing stdout here?

    > printf( "Enter day: " );


    fflush(stdout);

    > fgets( buffer, MAX, stdin );
    > money_log.day = atoi( buffer );


    You can't do that. money_log.day is an array, and you can't assign
    to an array.
    You should also be checking for valid input here, rather than just
    taking for granted that the user will enter the kind of data expected.

    > if( ( p = strchr( buffer, '\n' ) != NULL )

    ^
    At least you're consistent in your errors, if not your indentation
    style.

    > *p = '\0';
    > fflush( stdout );


    You need to flush after writing but before reading, not after
    everything.

    >/* I have been experimenting here (above) with different things, and I
    >get a pile of warnings on my compiler which I will post also. */
    >
    > printf ("Enter a description of the purchase (eg, phone bill):
    >");


    You need to be careful of line wrapping when posting code here. In
    theory, we should be able to copy & paste your code straight from your
    post, and compile it unaltered. In practice, however, there is usually
    some editing to be done to rejoin wrapped lines. It is your job, as
    the one asking for help, to ensure that we don't have to do this.

    fflush(stdout);

    > fgets(money_log.purchase_description, MAX, stdin);
    >
    > printf ("Enter the amount of the purchase : ");


    fflush(stdout);

    > fgets(buffer, MAX, stdin);
    > money_log.purchase = atoi( buffer );


    There are two errors here. First, money_log.purchase is an array,
    and you cannot assign to an array. Second, it's an array of float, and
    you're assigning it an int value. atoi() returns an int, oddly enough.

    > CLEAR;


    See above.

    > printf( "Category\tDate\tPurchase\tAmount\n\n" );
    > printf( "%s %s%d %s %.2f", money_log.category,
    >money_log.month, money_log.day, money_log.purchase_description,
    >money_log.purchase );


    And again you're forgetting that money_log.day and
    money_log.purchase are arrays.

    >}
    >
    >/* get_num() - get a number from the user, and check it for validity
    >*/
    >int get_num( void )
    >{
    > int choice;
    > char buffer[BUFSIZ];
    >
    > if( fgets ( buffer, sizeof buffer, stdin ) != NULL ){
    > buffer[strlen ( buffer ) - 1] = '\0';


    You need to be careful here, as a whole line may ne be read in
    (because the line entered is longer than the buffer), in which case
    there is no neline in the buffer at this point. So, if you remove
    buffer[strlen(buffer) - 1], you may be removing part of the entered
    text. In this case it probably doesn't matter, but in a real-world
    program it can matter a great deal. That's why we usually use strchr()
    to find the newline (if there is one).

    > if( validate( buffer ) == 0 ){
    > choice = atoi( buffer );
    > }
    > else
    > printf( "\nPlease enter a numerical character." );
    > }
    > else
    > printf( "Error reading input\n" );
    >
    > return choice;


    Note that choice has not been initialised, and still contains
    garbage, if the fgets() call failed.

    >}
    >
    >/* validate() - connected to get_num() */
    >int validate( char *a )
    >{
    > unsigned x;
    > for ( x = 0; x < strlen ( a ); x++ )
    > if( !isdigit ( a[x] ) ) return 1;
    >
    > return 0;
    >}
    >
    >Here are warnings gcc gives me when I try to compile:
    >
    >another.c: In function `add_record':
    >another.c:62: warning: assignment makes pointer from integer without a
    >cast
    >another.c:63: invalid operands to binary *


    This is an error, not a warning.

    >another.c:63: parse error before ';' token


    This is an error, not a warning.

    >another.c:70: warning: assignment makes pointer from integer without a
    >cast
    >another.c:71: invalid operands to binary *


    This is an error, not a warning.

    >another.c:71: parse error before ';' token


    This is an error, not a warning.

    >another.c:77: incompatible types in assignment


    This is an error, not a warning.

    >another.c:78: warning: assignment makes pointer from integer without a
    >cast
    >another.c:79: invalid operands to binary *


    This is an error, not a warning.

    >another.c:79: parse error before ';' token


    This is an error, not a warning.

    >another.c:88: incompatible types in assignment


    This is an error, not a warning.

    >another.c:92: warning: int format, pointer arg (arg 4)
    >another.c:92: warning: double format, pointer arg (arg 6)


    Well, you have alot more than just a few warnings. You have a number
    of fully fledged errors there.

    >The reason the newline is bothering me is that I would like the data
    >to print out in one line, instead of many. At any rate, if any of you
    >could set me straight on getting rid of the newline with fgets() in
    >this particular case, I would be grateful.


    You're almost doing it right. But you haven't been paying attention
    to the compiler. It is telling you what the problem is, and you're
    just not listening. It is telling you that you have invalid operands
    for binary * (ie., the multiplication operator), but you don't have
    binary *. You've confused the compiler. The only * operators you have
    are unary * (indirection operator). This should tell you that
    something is seriously amiss with the way these statements are being
    parsed by the compiler. And this should tell you that there's
    something amiss with the statements themselves, that you've most
    likely made a typographical error. (And you have.) Your compiler is
    also telling you that it found a parse error before a semicolon (on
    the same line), which means that it wasn't expecting a semicolon
    there. This is also a good indicator that something is wrong with the
    statement that is terminated by that semicolon (or some statement
    before it). Typos are very common, and even catch out the best
    programmers from time to time.
    Each of the last two warnings is about passing (the address of the
    first element of) an array to printf(). I have pointed out this error
    above.

    --

    Dig the even newer still, yet more improved, sig!

    http://alphalink.com.au/~phaywood/
    "Ain't I'm a dog?" - Ronny Self, Ain't I'm a Dog, written by G. Sherry & W. Walker.
    I know it's not "technically correct" English; but since when was rock & roll "technically correct"?
     
    Peter Shaggy Haywood, Nov 25, 2003
    #6
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. ThunderMusic

    How I'm I supposed to use string tables?

    ThunderMusic, Oct 15, 2004, in forum: ASP .Net
    Replies:
    1
    Views:
    453
    John Saunders
    Oct 15, 2004
  2. Simon Harvey
    Replies:
    5
    Views:
    452
    Scott M.
    Nov 16, 2003
  3. Jan Nielsen
    Replies:
    7
    Views:
    532
    Jan Nielsen
    Feb 8, 2005
  4. Replies:
    3
    Views:
    1,518
  5. Michael Goldshteyn
    Replies:
    5
    Views:
    374
    Victor Bazarov
    Jan 16, 2007
Loading...

Share This Page