Unseen error

[hpy_awad]

I wrote that example from a book and there is en error in the display
module that it does not showing all the records are entered in the
input module.
I traced with some printf statments without getting the solution . I
think the error in the display module loop condition.
Thanks

#include <stdio.h>
//-exercise 7.5.1 - Goal Statistics
struct football
{
char name[30];
char team[30];
int goals;
};
main ()
{
struct football player[100];
int option;
//initialize player array
init_player(player);
do
{

//Display menu of options ;
menu();
//Determine users requirements
menu_choice(&option);

//Perform users specified option
switch(option)
{
case 1: //Enter information for all players
player_input(player);
break;
case 2: //display table for all players
goal_table(player);
break;
case 3: //update specific information for a player
goal_update(player);
break;
case 4: //exit from program
break;
default://illegal input
printf ("\n\n This is not an available option\nAvailable options are
1,2,3,4");
}
} while (option !=4);
}

init_player(player)
//------------------
struct football player[];
{
int i,j;
for (i=0;i<100;++i)
{
for (j=0;j<30;++j)
{
player[i].name[j]=' ';
player[i].team[j]=' ';
}
player[i].goals=0;
}
}

menu()
//------------------
{
system ("clear");
printf("\n\n football system ");
printf("\n ------------------- ");
printf("\n\n 1- Enter information ");
printf("\n\n 2- Display table for all players");
printf("\n\n 3- Update specific information");
printf("\n\n 4- Exit from program ");
}
menu_choice(opt)
//------------------
int *opt;
{
printf("\n\n Select one of the above (1-4) : ");
scanf("%d",opt);

//clear screen
system("clear");
}
player_input(player)
//------------------
struct football player[];
{
int i;
char more;
i=-1;
do
{
++i;

//input players name
printf("Enter players name : ");
input_string(player[i].name);
//input players team
printf("Enter players team : ");
input_string(player[i].team);
//input number of goals scored
fflush(stdin);
printf("Enter number of goals scored: ");
scanf("%d",&player[i].goals);

printf("\n %-30s %-30s %4d\n",player[i].name,player[i].team,player[i].goals);
printf("\n %d I Value inside insert\n ",i);
//more players
if (i<99)
{
printf("More playersd to be entered (y/n)---->");
scanf("%s",&more);
}
} while (more == 'y' && i<99);

//Terminate players list
if (i<99) player[i+1].goals=-1;
}
input_string(alpha)
//------------------
char *alpha;
{
int i;
i=-1;

//Flush the keyboard buffer
fflush(stdin);
do
{
++i;

//input a character
alpha[i]=getchar();
} while (alpha[i] !='\n' && i<29);

//Terminate string
alpha[i]='\0';
}

goal_table(player) //Display table of goals scored
//------------------
struct football player[];
{
int i=0;
char cont;

//Output table headings
printf("\n\n Name Team Goals");
printf("\n\n ---- ---- -----");

do
{
//Output player information
printf("\n %d I Value inside display ",i);
printf("\n %-30s %-30s %4d\n",player[i].name,player[i].team,player[i].goals);
++i;
} while (i<=2);
printf("\n\n Press C to continue ");
scanf("%s",&cont);
}

goal_update(player) //update table of goals scored
//------------------
struct football player[];
{
char name[30];
int i,match,goal;

//input players name to be updated
printf("Enter name of player");
input_string(name);

//Find players record
i=0;
while ((player[i].goals!=-1)&&(i<100)&&(match=strcmp(name,player[i].name)
!=0))
++i;

//Input number of goals to be added
printf("Enter number of goals to be added to players account");
scanf("%d",&goal);

//Update players record
if (match==0)
player[i].goals=player[i].goals+goal;
else
printf("\n\n Player %s is not in the goal table \n",name);
}

[Leo Custodio]

As an advice, you should try to indent the code so it would be easy to read.
Also, there is not only one error on that code, but as my compiler shows, 2
pages of them. (GCC 3.3.2). You're not trying to run it under a C++
compiler, are you?

Hint: review it. ;)

Leo Custodio
al*********@hotmail.com
--
-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: 2.6.2

mQCNAz/nyswAAAEEAM1Jl14YqNlrUGmr4vh5OKGbDg5qiFnY/Ioqa5j5j9jlTsiH
7EJNlhIvu5OV223D0REUmWbFaKBQlnZAaDRRROb52YPuZ8NQfy u/C5zvTz8qubEx
jWn+nYryqKZxQsDwjntkNIMxx5n+QB7WhDltenCFE/VxYhsTa59EWqUqkz/RAAUR
tC5MZW9uYXJkbyBDLiBDdXN0b2RpbyA8YWxpZW5zcHJpdGVAaG 90bWFpbC5jb20+
=xAh5
-----END PGP PUBLIC KEY BLOCK-----

<hp******@yahoo.com> escreveu na mensagem
news:67*************************@posting.google.co m...

I wrote that example from a book and there is en error in the display
module that it does not showing all the records are entered in the
input module.
I traced with some printf statments without getting the solution . I
think the error in the display module loop condition.
Thanks

(skip...)

[Richard Heathfield]

[Followups set to acllcc++]

[Because you crossposted your article to (among other places) comp.lang.c,
this reply assumes you are programming in C.]

hp******@yahoo.com wrote:

#include <stdio.h>
//-exercise 7.5.1 - Goal Statistics
If you have a C99 compiler, // comments are legal. If you have a C90
compiler, they are not.
struct football
{
char name[30];
char team[30];
int goals;
};
main ()
If you have a C90 compiler, implicit int return type is legal. If you have a
C99 compiler, it is not. So, either way, you have a syntax problem.

<snip>
//input number of goals scored
fflush(stdin);
The behaviour of fflush on input streams is undefined.
printf("Enter number of goals scored: ");
scanf("%d",&player[i].goals);
What if the user types something that isn't a number?

printf("\n %-30s %-30s
%4d\n",player[i].name,player[i].team,player[i].goals);
printf("\n %d I Value inside insert\n ",i);
//more players
if (i<99)
{
printf("More playersd to be entered (y/n)---->");
scanf("%s",&more);

The 'more' is a single character. %s expects a pointer to an /array/ of
characters. You meant %c, not %s. In any event, an unadorned %s is unwise
(because of buffer overruns).

Fix those problems, and re-test. This will uncover more problems, but should
also lead to greater understanding.

Please pick your newsgroups more carefully next time. Thanks. (I think you
are best off in alt.comp.lang.learn.c-c++ at present, but comp.lang.c is a
decent alternative in this case. The other groups to which you posted are
really implementation groups, not language groups.)
--
Richard Heathfield : bi****@eton.powernet.co.uk
"Usenet is a strange place." - Dennis M Ritchie, 29 July 1999.
C FAQ: http://www.eskimo.com/~scs/C-faq/top.html
K&R answers, C books, etc: http://users.powernet.co.uk/eton

[darklight]

i am a begginer a list of errors
i got this when i compile using
gcc -Wall -ggdb filename.c -o filename
play1.c:10: warning: return type defaults to `int'
play1.c: In function `main':
play1.c:14: warning: implicit declaration of function `init_player'
play1.c:19: warning: implicit declaration of function `menu'
play1.c:23: warning: implicit declaration of function `menu_choice'
play1.c:29: warning: implicit declaration of function `player_input'
play1.c:32: warning: implicit declaration of function `goal_table'
play1.c:35: warning: implicit declaration of function `goal_update'
play1.c:43: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:47: warning: return type defaults to `int'
play1.c: In function `init_player':
play1.c:59: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:63: warning: return type defaults to `int'
play1.c: In function `menu':
play1.c:64: warning: implicit declaration of function `system'
play1.c:71: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:76: warning: return type defaults to `int'
play1.c: In function `menu_choice':
play1.c:83: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:88: warning: return type defaults to `int'
play1.c: In function `player_input':
play1.c:126: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:131: warning: return type defaults to `int'
play1.c: In function `input_string':
play1.c:148: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:152: warning: return type defaults to `int'
play1.c: In function `goal_table':
play1.c:170: warning: control reaches end of non-void function
play1.c: At top level:
play1.c:174: warning: return type defaults to `int'
play1.c: In function `goal_update':
play1.c:185: warning: implicit declaration of function `strcmp'
play1.c:198: warning: control reaches end of non-void function

[Alan Coopersmith]

hp******@yahoo.com (hp******@yahoo.com) writes in comp.unix.solaris:
|I wrote that example from a book and there is en error in the display
|module that it does not showing all the records are entered in the
|input module.
|I traced with some printf statments without getting the solution . I
|think the error in the display module loop condition.

Could that be because you limit the display to only the first two
records in the display module?
| } while (i<=2);

That seems incredibly obvious, especially since you identified that
as the possible problem.

|init_player(player)
|//------------------
|struct football player[];
If you're just learning C now, get a new book that teaches you ANSI C
(aka C89) and not the ancient K&R style which is long obsolete and will
only cause you problems with modern compilers.

| for (j=0;j<30;++j)
| {
| player[i].name[j]=' ';
| player[i].team[j]=' ';
| }

There's no '\0' at the end of those strings, so attempts to print them
may include random garbage at the end, or anything else. It would be
much better to simply do:
player[i].name[0] = '\0';
player[i].team[0] = '\0';

Or use memset() if you want to clear the entire string length to 0.

|do
|{
| ++i;
|
| //input a character
| alpha[i]=getchar();
|} while (alpha[i] !='\n' && i<29);

This could be written more simply with fgets()

--
__________________________________________________ ______________________
Alan Coopersmith al***@alum.calberkeley.org
http://www.CSUA.Berkeley.EDU/~alanc/ aka: Al**************@Sun.COM
Working for, but definitely not speaking for, Sun Microsystems, Inc.

[Barry Schwarz]

On 27 Dec 2003 07:05:12 -0800, hp******@yahoo.com (hp******@yahoo.com)
wrote:

I wrote that example from a book and there is en error in the display
Assuming you copied it correctly, the book has several errors. Any
chance it was written by Schildt?
module that it does not showing all the records are entered in the
input module.
I traced with some printf statments without getting the solution . I
think the error in the display module loop condition.
Thanks

#include <stdio.h>
//-exercise 7.5.1 - Goal Statistics
struct football
{
char name[30];
char team[30];
int goals;
};
main ()
While older compilers still accept this, the new language standard has
done away with implied return types for functions. Get in the habit
of doing it right:
int main(void)
{
struct football player[100];
int option;
//initialize player array
Interesting that your book uses a comment style allowed only in C99
yet uses other features (such as above) that are disallowed in C99.
init_player(player);
do
{

//Display menu of options ;
menu();
//Determine users requirements
menu_choice(&option);

//Perform users specified option
switch(option)
{
case 1: //Enter information for all players
player_input(player);
break;
case 2: //display table for all players
goal_table(player);
break;
case 3: //update specific information for a player
goal_update(player);
break;
case 4: //exit from program
break;
default://illegal input
printf ("\n\n This is not an available option\nAvailable options are
1,2,3,4");
Any time your interactive output does not end with a '\n', you run the
risk of it not being displayed to the user due to buffering
considerations.
}
} while (option !=4);
Not an error but most recommend an explicit return from main().
}

init_player(player)
//------------------
struct football player[];
Since this is not main and since you don't return a value, the implied
return type of int here is not acceptable. You must specify void.

While still legal, this construction is very obsolete. (When was the
book written?) The "modern" construction combines the two lines to
void init_player(struct football player[])

This has the additional benefit of allowing you to place function
prototypes in scope before you call the function. I believe this is a
requirement in the new language standard but is a good idea even if
not since it allows the compiler to check that your arguments to the
function have the correct type.
{
int i,j;
for (i=0;i<100;++i)
{
for (j=0;j<30;++j)
{
player[i].name[j]=' ';
player[i].team[j]=' ';
}
You do realize that this serves no purpose?
player[i].goals=0;
Neither does this.
}
}

menu()
//------------------
{
system ("clear");
system is declared in stdlib.h which you did not #include.
printf("\n\n football system ");
printf("\n ------------------- ");
printf("\n\n 1- Enter information ");
printf("\n\n 2- Display table for all players");
printf("\n\n 3- Update specific information");
printf("\n\n 4- Exit from program ");
}
menu_choice(opt)
//------------------
int *opt;
{
printf("\n\n Select one of the above (1-4) : ");
scanf("%d",opt);

//clear screen
system("clear");
Why on earth would you want to clear the screen right after accepting
user input?
}
player_input(player)
//------------------
struct football player[];
{
int i;
char more;
i=-1;
do
{
++i;

//input players name
printf("Enter players name : ");
input_string(player[i].name);
//input players team
printf("Enter players team : ");
input_string(player[i].team);
//input number of goals scored
fflush(stdin);
This invokes undefined behavior. fflush is defined for output streams
only and has never been defined for input streams.
printf("Enter number of goals scored: ");
scanf("%d",&player[i].goals);

printf("\n %-30s %-30s %4d\n",player[i].name,player[i].team,player[i].goals);
printf("\n %d I Value inside insert\n ",i);
//more players
if (i<99)
{
printf("More playersd to be entered (y/n)---->");
scanf("%s",&more);
This is a major problem. more is a single char. %s will accept a
string which is guaranteed to be longer than one char. This invokes
undefined behavior and overwrites whatever is in memory following
more. One possible solution is to use %c.
}
} while (more == 'y' && i<99);
What if the user types 'Y'?

//Terminate players list
if (i<99) player[i+1].goals=-1;
}
input_string(alpha)
//------------------
char *alpha;
{
int i;
i=-1;

//Flush the keyboard buffer
fflush(stdin);
do
{
++i;

//input a character
alpha[i]=getchar();
} while (alpha[i] !='\n' && i<29);
When i is 28, the while is true and you loop one final time to accept
alpha[29] ...
//Terminate string
alpha[i]='\0';
and you then overlay alpha[29] with a nul. It is bad form to destroy
a user's input without telling him. If you coded 28 in the while, he
would at least know that you didn't let him enter the last character.
}

goal_table(player) //Display table of goals scored
//------------------
struct football player[];
{
int i=0;
char cont;

//Output table headings
printf("\n\n Name Team Goals");
printf("\n\n ---- ---- -----");
These titles and underscores are not wide enough for the 30 character
entries that follow.

do
{
//Output player information
printf("\n %d I Value inside display ",i);
printf("\n %-30s %-30s %4d\n",player[i].name,player[i].team,player[i].goals);
++i;
} while (i<=2);
This is the source of the error you asked about. Did you mean 99
here? 2 makes no sense at all. This looks like a transcription
error.

Even with 99, there is a logic error. In init_player(), you
initialize name and team to blanks with no terminating '\0'. In
player_input(), you allow for the possibility of less than 100
players. This loop does not test for this and will attempt to print
names and teams that are not strings with the %s format. This invokes
undefined behavior.
printf("\n\n Press C to continue ");
scanf("%s",&cont);
Another attempt to read a string into a single char. Is this really
what the book says?
}

goal_update(player) //update table of goals scored
//------------------
struct football player[];
{
char name[30];
int i,match,goal;

//input players name to be updated
printf("Enter name of player");
input_string(name);

//Find players record
i=0;
while ((player[i].goals!=-1)&&(i<100)&&(match=strcmp(name,player[i].name)
!=0))
Even though it produces the correct result in most cases, this is a
poor construct. To see why, let's reformat it so the news reader does
not break up the line. The only changes I am making are removing the
/n> that indicates quoted material, inserting a space before each &&,
and inserting a \n after each &&.
while ((player[i].goals!=-1) &&
(i<100) &&
(match=strcmp(name,player[i].name)!=0))

The first problem is the second test needs to be first. You cannot
check player[i].goals if i is 100 or more. That variable does not
exist and attempting to do so invokes undefined behavior. The &&
operator has a short-circuit evaluation so if you rearrange the tests
and the first one fails, the remaining are never evaluated.

Additionally, last expression is "broken." Here it is again with some
addition (and suggestive) white space.
(match = strcmp(name,player[i].name) != 0)

Since != has higher precedence than =, this expression is evaluated as
( match = (strcmp(name,player[i].name) != 0) )
which means "assign to match the value 1 or 0 depending on whether the
return value from strcmp is different from or the same as 0,
respectively."

The recommended construction is
( (match = strcmp(name,player[i].name)) != 0 )
which means "assign the return value from strcmp to match and then
determine if it is different from or the same as 0."

The reason yours works in this case is because you are only interested
in equality or inequality and match will be 0 if and only if strcmp
returns 0. However, for example, if your array were sorted, you would
be interested in two of the three possible return values from strcmp.
Your construction would not give you that. Both negative and positive
returns from strcmp would be combined into a single value for match.
++i;

//Input number of goals to be added
printf("Enter number of goals to be added to players account");
scanf("%d",&goal);
Don't you think this would be better inside the following if? Why ask
for input if you know you cannot use it?

//Update players record
if (match==0)
player[i].goals=player[i].goals+goal;
else
printf("\n\n Player %s is not in the goal table \n",name);
}

If you give us the name of the book and author and date, we can add it
to the list of books not to use.
<<Remove the del for email>>