Won't work without useless declarations

Discussion in 'C Programming' started by Lars Eighner, Sep 16, 2007.

  1. Lars Eighner

    Lars Eighner Guest

    In the code below, the line "unsigned int ktop,kbot;"
    seems to be useless as ktop and kbot are never used. Yet,
    this work with it and breaks without it.

    Any idea why?




    #include <stdio.h>
    #include <termios.h>
    #include <string.h>


    main ()
    {

    getkey();

    }


    getkey ()
    {
    struct termios unwhacked, whacked;
    unsigned int ktop,kbot;
    char *k;
    char kstr[10];
    tcflag_t aswas;
    fflush(0);
    tcgetattr(0,&unwhacked); /* get terminal state */
    whacked = unwhacked; /* save state for restore */
    whacked.c_lflag &= ~ICANON; /* turn off cannical input */
    whacked.c_lflag &= ~ECHO; /* turn off echoing */
    whacked.c_cc[VMIN] = 1; /* capture at least 1 character */
    whacked.c_cc[VTIME] = 1; /* and of them that come quick */
    tcsetattr(0,TCSANOW,&whacked); /* whack the terminal with new flags now */
    read (0,&k,5);
    printf ("value is %x \n",k);
    sprintf(kstr,"%x",k);
    printf("%s length is = %i",kstr,strlen(kstr));
    if(strlen(kstr) == 2){ /* not a function string */
    if( kstr[0] > '1' && kstr[0] < '8'){
    if (kstr[0] == '2' && kstr[1] == '0')
    {printf (" letter is <space>");}else
    /* yeah, it is printable ASCII sort of */
    if (kstr[0] == '7' && kstr[1] == 'f')
    {printf (" letter is <^? DEL>");}else
    if(kstr[0] >= '2')
    {printf (" letter is %c",k);}
    /* gets the printable ASCII characters */
    }
    if (kstr[0] > '9')
    {printf (" letter is %c",k);} /* printable 8 bit characters */


    }

    tcsetattr(0,TCSANOW,&unwhacked); /* unwhack the terminal */
    return 0;
    }



    --
    Lars Eighner <http://larseighner.com/> <http://myspace.com/larseighner>
    Countdown: 492 days to go.
    What do you do when you're debranded?
    Lars Eighner, Sep 16, 2007
    #1
    1. Advertising

  2. On Sep 15, 11:18 pm, Lars Eighner <> wrote:
    > In the code below, the line "unsigned int ktop,kbot;"
    > seems to be useless as ktop and kbot are never used. Yet,
    > this work with it and breaks without it.
    >
    > Any idea why?
    >
    > #include <stdio.h>
    > #include <termios.h>
    > #include <string.h>
    >
    > main ()
    > {
    >
    > getkey();
    >
    > }
    >
    > getkey ()
    > {
    > struct termios unwhacked, whacked;
    > unsigned int ktop,kbot;
    > char *k;
    > char kstr[10];
    > tcflag_t aswas;
    > fflush(0);
    > tcgetattr(0,&unwhacked); /* get terminal state */
    > whacked = unwhacked; /* save state for restore */
    > whacked.c_lflag &= ~ICANON; /* turn off cannical input */
    > whacked.c_lflag &= ~ECHO; /* turn off echoing */
    > whacked.c_cc[VMIN] = 1; /* capture at least 1 character */
    > whacked.c_cc[VTIME] = 1; /* and of them that come quick */
    > tcsetattr(0,TCSANOW,&whacked); /* whack the terminal with new flags now */
    > read (0,&k,5);


    (Note: read is not standard C) At this point you try and put
    5 characters into a pointer to char. A pointer to char will
    (probably) hold 4 characters. Undefined behaviour. Anything can
    happen.

    What is probably happening, is that if the "useless" declarations
    are made, there is space in the stack above k, so the fifth character
    does
    not overwrite anything of importance. If the "useless" declarations
    are not made then the fifth character does overwrite something
    important.


    > printf ("value is %x \n",k);
    > sprintf(kstr,"%x",k);


    k is not known to be a string. This may do horrible things.
    Whatever you do, do not try this code on a DS2K.

    - William Hughes
    William Hughes, Sep 16, 2007
    #2
    1. Advertising

  3. [comp.lang.c] Lars Eighner <> wrote:

    > In the code below, the line "unsigned int ktop,kbot;"
    > seems to be useless as ktop and kbot are never used. Yet,
    > this work with it and breaks without it.


    You'll get better answers on comp.unix.programmer. Consider, however,
    the following altered version of your code, which fixes many obvious
    problems (and compiles without warnings):

    #include <stdio.h>
    #include <termios.h>
    #include <string.h>
    #include <unistd.h> /* OT - required for read() */

    int getkey(); /* prototype is a very good idea */

    int main( void ) { /* implicit int is a bad idea */
    return getkey(); /* need to return something */
    }

    int getkey () { /* explicit int again */
    struct termios unwhacked, whacked;
    /* unsigned int ktop,kbot; */
    char *k;
    char kstr[10];
    /* itcflag_t aswas; */ /* seems completely superfluous */
    fflush(0);
    tcgetattr(0,&unwhacked); /* get terminal state */
    whacked = unwhacked; /* save state for restore */
    whacked.c_lflag &= ~ICANON; /* turn off cannical input */
    whacked.c_lflag &= ~ECHO; /* turn off echoing */
    whacked.c_cc[VMIN] = 1; /* capture at least 1 character */
    whacked.c_cc[VTIME] = 1; /* and of them that come quick */
    tcsetattr(0,TCSANOW,&whacked); /* whack the terminal with new flags now */
    read (0,&k,5);
    printf ("value is %p \n",(void*)k); /* correct way to print a
    pointer value */
    sprintf(kstr,"%p",(void*)k); /* ditto */
    /* correct way to print variables of type size_t */
    printf("%s length is = %lu",kstr,(unsigned long)strlen(kstr));
    if(strlen(kstr) == 2) { /* not a function string */
    if( kstr[0] > '1' && kstr[0] < '8'){
    if (kstr[0] == '2' && kstr[1] == '0') {
    printf (" letter is <space>");
    }
    else if (kstr[0] == '7' && kstr[1] == 'f') {
    printf (" letter is <^? DEL>");
    } else if(kstr[0] >= '2') {
    printf (" letter is %c",*k); /* k is a char *, not a char */
    }
    }
    if (kstr[0] > '9') {
    printf (" letter is %c",*k); /* ditto */
    } /* printable 8 bit characters */
    }
    tcsetattr(0,TCSANOW,&unwhacked); /* unwhack the terminal */
    printf( "\n" ); /* a good idea */
    return 0;
    }

    This still doesn't seem to do what's intended, but at least you can
    rule out the possibility that one of the other errors was somehow to
    blame.

    --
    C. Benson Manica | I appreciate all corrections, polite or otherwise.
    cbmanica(at)gmail.com |
    ----------------------| I do not currently read any posts posted through
    sdf.lonestar.org | Google groups, due to rampant unchecked spam.
    Christopher Benson-Manica, Sep 17, 2007
    #3
  4. Lars Eighner

    John Bode Guest

    On Sep 15, 10:18 pm, Lars Eighner <> wrote:
    > In the code below, the line "unsigned int ktop,kbot;"
    > seems to be useless as ktop and kbot are never used. Yet,
    > this work with it and breaks without it.
    >
    > Any idea why?
    >
    > #include <stdio.h>
    > #include <termios.h>
    > #include <string.h>
    >
    > main ()


    This isn't your problem, but avoid implicit declarations in the future
    (I think the latest version of the C standard explicitly disallows
    implicit declarations for main(), but I could be wrong). Use fully-
    typed, prototype-style declarations in the future:

    int main(void)
    > {
    >
    > getkey();
    >
    > }
    >
    > getkey ()


    Similarly for this. Implicit declarations *will* bite you in the ass
    eventually.

    int getkey(void)
    > {
    > struct termios unwhacked, whacked;
    > unsigned int ktop,kbot;
    > char *k;


    [snip]

    > read (0,&k,5);


    This is your problem right here. At this point, k hasn't been
    initialized to point anywhere meaningful; you've invoked undefined
    behavior by writing to a random memory address, which in this case
    appears to be the memory immediately preceding k itself. Bad juju.

    You would be better off making k a static array instead of a pointer:

    char k[5]; // or however big k needs to be
    ....
    read(0, k, sizeof k);

    Note that you don't need to use the address-of (&) operator on k, as
    an array identifier in this context is converted to a pointer type.

    Oh, and the line

    fflush(0);

    is a problem; fflush() is not defined for input streams (the operation
    is meaningless for input). Lose it.
    John Bode, Sep 17, 2007
    #4
  5. "John Bode" <> a écrit dans le message de news:
    ...
    > On Sep 15, 10:18 pm, Lars Eighner <> wrote:
    >> In the code below, the line "unsigned int ktop,kbot;"
    >> seems to be useless as ktop and kbot are never used. Yet,
    >> this work with it and breaks without it.
    >>
    >> Any idea why?
    >>

    <snip>

    > int getkey(void)
    >> {
    >> struct termios unwhacked, whacked;
    >> unsigned int ktop,kbot;
    >> char *k;

    >
    > [snip]
    >
    >> read (0,&k,5);

    >
    > This is your problem right here. At this point, k hasn't been
    > initialized to point anywhere meaningful; you've invoked undefined
    > behavior by writing to a random memory address, which in this case
    > appears to be the memory immediately preceding k itself. Bad juju.


    close, but no cigar. OP passes the address of the uninitialized pointer k
    and a length of 5. read will attempt to read 5 bytes into the pointer
    itself and whatever follows. If pointers are 32 bits, some other variable
    will be clobbered (in this case the unused ktop or kbot) or even worse, the
    return address or the saved frame pointer will be changed causing undefined
    behaviour (crash is likely). This line is definitely what is causing the
    problem and this is the explaination for the curious side effect of removing
    the unused variables.

    > You would be better off making k a static array instead of a pointer:
    >
    > char k[5]; // or however big k needs to be
    > ...
    > read(0, k, sizeof k);
    >
    > Note that you don't need to use the address-of (&) operator on k, as
    > an array identifier in this context is converted to a pointer type.


    Yes that is a good fix. Except for the vocabulary: char k[5] is by no means
    a 'static array', it is an array with automatic storage, as such it is
    uninitialized.
    read's return value should be checked to verify how many bytes have actually
    been read.

    > Oh, and the line
    >
    > fflush(0);
    >
    > is a problem; fflush() is not defined for input streams (the operation
    > is meaningless for input). Lose it.


    Again, good point, but irrelevant.
    fflush(stdin) is useless, or at best implementation defined.
    fflush(0) or equivalently fflush(NULL) flushes all FILEs currently open for
    output.

    --
    Chqrlie.
    Charlie Gordon, Sep 17, 2007
    #5
  6. Lars Eighner

    CBFalconer Guest

    Christopher Benson-Manica wrote:
    > Lars Eighner <> wrote:
    >
    >> In the code below, the line "unsigned int ktop,kbot;"
    >> seems to be useless as ktop and kbot are never used. Yet,
    >> this work with it and breaks without it.

    >
    > You'll get better answers on comp.unix.programmer. Consider,
    > however, the following altered version of your code, which fixes
    > many obvious problems (and compiles without warnings):


    It's still illegal standard C coding. Some of the faults are
    detailed below.

    >
    > #include <stdio.h>
    > #include <termios.h>


    No such include file.

    > #include <string.h>
    > #include <unistd.h> /* OT - required for read() */


    No such include file. No such std routine as read.

    >
    > int getkey(); /* prototype is a very good idea */
    >
    > int main( void ) { /* implicit int is a bad idea */
    > return getkey(); /* need to return something */


    Probably a bad value to return.

    ....Rest of code snipped...

    --
    Chuck F (cbfalconer at maineline dot net)
    Available for consulting/temporary embedded and systems.
    <http://cbfalconer.home.att.net>



    --
    Posted via a free Usenet account from http://www.teranews.com
    CBFalconer, Sep 17, 2007
    #6
  7. John Bode <> writes:
    > On Sep 15, 10:18 pm, Lars Eighner <> wrote:
    >> In the code below, the line "unsigned int ktop,kbot;"
    >> seems to be useless as ktop and kbot are never used. Yet,
    >> this work with it and breaks without it.
    >>
    >> Any idea why?
    >>
    >> #include <stdio.h>
    >> #include <termios.h>
    >> #include <string.h>
    >>
    >> main ()

    >
    > This isn't your problem, but avoid implicit declarations in the future
    > (I think the latest version of the C standard explicitly disallows
    > implicit declarations for main(), but I could be wrong). Use fully-
    > typed, prototype-style declarations in the future:
    >
    > int main(void)

    [snip]

    A minor quibble: That's not an implicit declaration for main. It's an
    explicit declaration (part of a definition) that uses implicit int.

    But since C99 forbids both implicit function declarations and implicit
    int, and depending on them is poor style even in C90, the change is a
    good idea.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    San Diego Supercomputer Center <*> <http://users.sdsc.edu/~kst>
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
    Keith Thompson, Sep 17, 2007
    #7
  8. "CBFalconer" <> a écrit dans le message de news:
    ...
    > Christopher Benson-Manica wrote:
    >> Lars Eighner <> wrote:
    >>
    >>> In the code below, the line "unsigned int ktop,kbot;"
    >>> seems to be useless as ktop and kbot are never used. Yet,
    >>> this work with it and breaks without it.

    >>
    >> You'll get better answers on comp.unix.programmer. Consider,
    >> however, the following altered version of your code, which fixes
    >> many obvious problems (and compiles without warnings):

    >
    > It's still illegal standard C coding. Some of the faults are
    > detailed below.
    >
    >>
    >> #include <stdio.h>
    >> #include <termios.h>

    >
    > No such include file.
    >
    >> #include <string.h>
    >> #include <unistd.h> /* OT - required for read() */

    >
    > No such include file. No such std routine as read.
    >


    What makes it not Standard C code ?
    This is a program that uses implementation specific header files and library
    functions, to which source or declarations were not given, but are largely
    irrelevant to to OPs question. The bug was already found and explained,
    without your help, why keep bashing people unnecessarily.

    >>
    >> int getkey(); /* prototype is a very good idea */


    Excellent point.

    >> int main( void ) { /* implicit int is a bad idea */
    >> return getkey(); /* need to return something */

    >
    > Probably a bad value to return.


    I agree.

    We seem to share the same intuitions ;-)

    --
    Chqrlie.
    Charlie Gordon, Sep 18, 2007
    #8
  9. [comp.lang.c] CBFalconer <> wrote:

    > Christopher Benson-Manica wrote:


    >> #include <unistd.h> /* OT - required for read() */


    > No such include file. No such std routine as read.


    I did note that it was OT, did I not?

    >> return getkey(); /* need to return something */


    > Probably a bad value to return.


    This I agree with; I thought better of it after I had posted.

    --
    C. Benson Manica | I appreciate all corrections, polite or otherwise.
    cbmanica(at)gmail.com |
    ----------------------| I do not currently read any posts posted through
    sdf.lonestar.org | Google groups, due to rampant unchecked spam.
    Christopher Benson-Manica, Sep 18, 2007
    #9
  10. On Sep 18, 6:22 am, "Charlie Gordon" <> wrote:
    > "CBFalconer" <> a écrit dans le message de news:
    > ...
    >
    >
    >
    > > Christopher Benson-Manica wrote:
    > >> Lars Eighner <> wrote:

    >
    > >>> In the code below, the line "unsigned int ktop,kbot;"
    > >>> seems to be useless as ktop and kbot are never used. Yet,
    > >>> this work with it and breaks without it.

    >
    > >> You'll get better answers on comp.unix.programmer. Consider,
    > >> however, the following altered version of your code, which fixes
    > >> many obvious problems (and compiles without warnings):

    >
    > > It's still illegal standard C coding. Some of the faults are
    > > detailed below.

    >
    > >> #include <stdio.h>
    > >> #include <termios.h>

    >
    > > No such include file.

    >
    > >> #include <string.h>
    > >> #include <unistd.h> /* OT - required for read() */

    >
    > > No such include file. No such std routine as read.

    >
    > What makes it not Standard C code ?
    > This is a program that uses implementation specific header files and library
    > functions, to which source or declarations were not given,


    This is not Standard C code becaue it uses implementation specific
    header files and non-standard functions for which source
    is not given,

    > but are largely irrelevant to to OPs question.


    This is totally irrelevant to the
    question of whether this is Standard C code

    > The bug was already found and explained,


    And, as noted, the bug involved a non-standard function.


    - William Hughes
    William Hughes, Sep 18, 2007
    #10
    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. Chad
    Replies:
    4
    Views:
    8,314
  2. Daniel Nogradi
    Replies:
    0
    Views:
    378
    Daniel Nogradi
    Nov 15, 2006
  3. Replies:
    11
    Views:
    803
    akspring
    Sep 21, 2013
  4. davehigton
    Replies:
    2
    Views:
    342
    davehigton
    Dec 24, 2008
  5. Peter
    Replies:
    4
    Views:
    63
Loading...

Share This Page