fgets() - supposed to be simple :)

[Rob Somers]

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.

[Kevin Goodsell]

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.

[Kurt Watzka]

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

[Rob Somers]

Kurt Watzka <wa****@stat.uni-muenchen.de> wrote in message news:<bp*************@news.t-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

[Malcolm]

"Rob Somers" <ke*****@gto.net> 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.

[Peter Shaggy Haywood]

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"?