Unseen error

Discussion in 'C Programming' started by hpy_awad@yahoo.com, Dec 27, 2003.

  1. Guest

    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.name[j]=' ';
    player.team[j]=' ';
    }
    player.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.name);


    //input players team
    printf("Enter players team : ");
    input_string(player.team);


    //input number of goals scored
    fflush(stdin);
    printf("Enter number of goals scored: ");
    scanf("%d",&player.goals);

    printf("\n %-30s %-30s %4d\n",player.name,player.team,player.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=getchar();
    } while (alpha !='\n' && i<29);

    //Terminate string
    alpha='\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.name,player.team,player.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.goals!=-1)&&(i<100)&&(match=strcmp(name,player.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.goals=player.goals+goal;
    else
    printf("\n\n Player %s is not in the goal table \n",name);
    }
     
    , Dec 27, 2003
    #1
    1. Advertising

  2. Leo Custodio Guest

    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

    --
    -----BEGIN PGP PUBLIC KEY BLOCK-----
    Version: 2.6.2

    mQCNAz/nyswAAAEEAM1Jl14YqNlrUGmr4vh5OKGbDg5qiFnY/Ioqa5j5j9jlTsiH
    7EJNlhIvu5OV223D0REUmWbFaKBQlnZAaDRRROb52YPuZ8NQfyu/C5zvTz8qubEx
    jWn+nYryqKZxQsDwjntkNIMxx5n+QB7WhDltenCFE/VxYhsTa59EWqUqkz/RAAUR
    tC5MZW9uYXJkbyBDLiBDdXN0b2RpbyA8YWxpZW5zcHJpdGVAaG90bWFpbC5jb20+
    =xAh5
    -----END PGP PUBLIC KEY BLOCK-----

    <> escreveu na mensagem
    news:...
    > 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...)
     
    Leo Custodio, Dec 27, 2003
    #2
    1. Advertising

  3. [Followups set to acllcc++]

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

    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.goals);


    What if the user types something that isn't a number?

    >
    > printf("\n %-30s %-30s
    > %4d\n",player.name,player.team,player.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 :
    "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
     
    Richard Heathfield, Dec 27, 2003
    #3
  4. darklight Guest

    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
     
    darklight, Dec 27, 2003
    #4
  5. () 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.name[j]=' ';
    | player.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.name[0] = '\0';
    player.team[0] = '\0';

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

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

    This could be written more simply with fgets()

    --
    ________________________________________________________________________
    Alan Coopersmith
    http://www.CSUA.Berkeley.EDU/~alanc/ aka:
    Working for, but definitely not speaking for, Sun Microsystems, Inc.
     
    Alan Coopersmith, Dec 27, 2003
    #5
  6. On 27 Dec 2003 07:05:12 -0800, ()
    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.name[j]=' ';
    > player.team[j]=' ';
    > }


    You do realize that this serves no purpose?

    > player.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.name);
    >
    >
    > //input players team
    > printf("Enter players team : ");
    > input_string(player.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.goals);
    >
    > printf("\n %-30s %-30s %4d\n",player.name,player.team,player.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=getchar();
    >} while (alpha !='\n' && i<29);


    When i is 28, the while is true and you loop one final time to accept
    alpha[29] ...
    >
    >//Terminate string
    >alpha='\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.name,player.team,player.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.goals!=-1)&&(i<100)&&(match=strcmp(name,player.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.goals!=-1) &&
    (i<100) &&
    (match=strcmp(name,player.name)!=0))

    The first problem is the second test needs to be first. You cannot
    check player.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.
    [color=blue]
    >++i;
    >
    >//Input number of goals to be added
    >printf("Enter number of goals to be added to players account");
    >scanf("%d",&goal);[/color]

    Don't you think this would be better inside the following if? Why ask
    for input if you know you cannot use it?
    [color=blue]
    >
    >//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);
    >}[/i][/i][/color][i][i]

    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>>[/i][/i][/i][/i][/i]
     
    Barry Schwarz, Dec 27, 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. hfk0
    Replies:
    2
    Views:
    21,732
  2. JavaQueries
    Replies:
    1
    Views:
    3,752
    John C. Bollinger
    Mar 1, 2005
  3. Garvin, Michael [CAR:C28G:EXCH]

    [Q] Impact of Unseen Types

    Garvin, Michael [CAR:C28G:EXCH], Oct 18, 2004, in forum: C++
    Replies:
    3
    Views:
    323
    Garvin, Michael [CAR:C28G:EXCH]
    Oct 18, 2004
  4. freakrobot
    Replies:
    1
    Views:
    485
    Chris Rebert
    Mar 16, 2010
  5. Haircuts Are Important

    How To Learn Unseen 1553 Library Routines

    Haircuts Are Important, Apr 4, 2012, in forum: C Programming
    Replies:
    2
    Views:
    328
    ImpalerCore
    Apr 4, 2012
Loading...

Share This Page