Won't work without useless declarations

L

Lars Eighner

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;
}
 
W

William Hughes

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
 
C

Christopher Benson-Manica

[comp.lang.c] Lars Eighner said:
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.
 
J

John Bode

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.
 
C

Charlie Gordon

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.
 
C

CBFalconer

Christopher said:
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...
 
K

Keith Thompson

John Bode said:
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.
 
C

Charlie Gordon

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


No such include file.


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.

Excellent point.
Probably a bad value to return.

I agree.

We seem to share the same intuitions ;-)
 
W

William Hughes

"CBFalconer" <[email protected]> a écrit dans le message de (e-mail address removed)...







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
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Fibonacci 0
C language. work with text 3
Cast problem / serial interface 5
Help regarding Programming Style 4
termios help 1
Help using termios 2
Programming serial interface question (linux) 3
Termios is freezing 6

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,482
Members
44,901
Latest member
Noble71S45

Latest Threads

Top