|
|
 |
 |
 |
 |
strcmp problems.... ??? Segmentation fault
Hi All here is a problem I just can't understand! I don't really want a work around I just want this to plainly work or atleast explained.... Thanks in advance.... Problem: strcmp(st[i], s) it runs with no problem.... when I put this in an if statement or set a integer equal this it makes the program crash... here is the code, last function is were it dies... #include <stdio.h> #include <string.h> enum { MAX_LINES = 20, MAX_LINE_LEN = 80 };
char buff[ MAX_LINE_LEN ]; char* st[ MAX_LINES ] = {NULL}; int numLines = 0; /* prompts user for search string, stores it in s */ /* s must be of sufficient size. */ void getInput( char *s ); /* returns TRUE if s appears in table */ int inTable( const char *s ); int main( int argc, char *argv[] ) { FILE *fin = NULL; /* load table */ if( argc < 2 ) { fprintf( stdout, "\nOkay, you didn't list an input file.\n" ); return -1; #if 0 // this is the code that should replace the return. Not tested fprintf( stdout, "So, type entries here, one per line. (ctrl-D to end)\$ fin = stdin; #endif } else { fin = fopen( argv[1], "r" ); if( fin==NULL ) { fprintf( stderr, "\nCan't open %s for reading. Exiting$ argv[1] ); return -1; } } while( numLines<MAX_LINES && fgets( buff, MAX_LINE_LEN, fin )) { ++numLines; st[ numLines ] = strdup( buff ); } fclose( fin ); /* get target */ getInput( buff ); if( inTable( buff )) printf( "\nYep, got one of those.\n" ); else printf( "\nSorry, check back next Sat.\n" ); return 0; }
void getInput( char *s ) { printf( "\nEnter a string to search for => " ); fgets( s, MAX_LINE_LEN, stdin ); }
int inTable( const char *s ) { int i; for( i=0; i<numLines; ++i ) { printf("Values s=%s",st[i]); if( strcmp(st[i], s) == 0 ) return 1; } return 0;
}
kevin wrote: > char* st[ MAX_LINES ] = {NULL}; > int numLines = 0;
[snip] > while( numLines<MAX_LINES && fgets( buff, MAX_LINE_LEN, fin )) > { > ++numLines; Increase numLines > st[ numLines ] = strdup( buff );
Then assign a copy of your string [snip] > int i; > for( i=0; i<numLines; ++i ) Start at index 0 > { > printf("Values s=%s",st[i]); > if( strcmp(st[i], s) == 0 ) > return 1; > }
[snip] I think your problem is that st[0] is NULL since you increase the index *before* you assign the copied string value.
On Sat, 02 Jun 2007 09:55:10 -0700, kevin <medicalsou@hotmail.com> wrote: >Hi All here is a problem I just can't understand! I don't really want >a work around I just want this to plainly work or atleast >explained.... Thanks in advance.... >Problem: strcmp(st[i], s) it runs with no problem.... when I put this >in an if statement or set a integer equal this it makes the program >crash...
What makes the program crash is invoking undefined behavior. The fact that it appears to work sometimes is just bad luck (or poor system design about which you don't have much control). >here is the code, last function is were it dies... >#include <stdio.h> >#include <string.h> >enum { > MAX_LINES = 20, > MAX_LINE_LEN = 80 >}; >char buff[ MAX_LINE_LEN ]; >char* st[ MAX_LINES ] = {NULL}; >int numLines = 0;
Both initializations are superfluous since objects at file scope are initialized automatically to these values.
>/* prompts user for search string, stores it in s */ >/* s must be of sufficient size. */ >void getInput( char *s ); >/* returns TRUE if s appears in table */ >int inTable( const char *s ); >int main( int argc, char *argv[] ) >{ > FILE *fin = NULL; > /* load table */ > if( argc < 2 ) > { > fprintf( stdout, "\nOkay, you didn't list an input >file.\n" ); > return -1;
If you want other people to compile and test your code so they can try to help you, don't use non-portable constructs which may cause problems with their systems. In this case, the standard provides EXIT_FAILURE as a portable return value from main.
>#if 0 // this is the code that should replace the return. Not >tested > fprintf( stdout, > "So, type entries here, one per line. (ctrl-D >to end)\$ > fin = stdin; >#endif > } > else > { > fin = fopen( argv[1], "r" ); > if( fin==NULL ) > { > fprintf( stderr, "\nCan't open %s for >reading. Exiting$ > argv[1] );
I give up. Where is the closing quote for the format string and the comma that separates the second and third arguments? More to the point, where is your real code? Use cut and paste, don't retype. > return -1; > } > } > while( numLines<MAX_LINES && fgets( buff, MAX_LINE_LEN, fin )) > { > ++numLines; > st[ numLines ] = strdup( buff );
strdup is a non-standard function. By incrementing numLines prior to using it, you never store a value in st[0]. Worse, when numLines was exactly MAX_LINES-1, you invoke undefined behavior by storing into a non-existent array element.
> } > fclose( fin ); > /* get target */ > getInput( buff ); > if( inTable( buff )) > printf( "\nYep, got one of those.\n" ); > else > printf( "\nSorry, check back next Sat.\n" ); > return 0; >} >void getInput( char *s ) >{ > printf( "\nEnter a string to search for => " );
Your text does not end with a \n. On some buffered systems, this prompt may not appear until after the user enters data. If you want the user's input on the same line as the prompt, add fflush(stdout); after this statement. > fgets( s, MAX_LINE_LEN, stdin ); >} >int inTable( const char *s ) >{ > int i; > for( i=0; i<numLines; ++i ) > { > printf("Values s=%s",st[i]);
Here you invoke undefined behavior when i is 0 since st[0] is still NULL. Didn't you notice garbage in your output? > if( strcmp(st[i], s) == 0 )
More undefined behavior for the same reason. > return 1; > } > return 0; >}
Remove del for email
Thank you for your help! I'm currently working on UNIX... that is why my code is written as is... while( numLines<MAX_LINES && fgets( buff, MAX_LINE_LEN, fin )) { > ++numLines; > st[ numLines ] = strdup( buff );
///The problem was def st[0] having trash... and fixed it by changing the while loop as sugested... Thanks for the help, have a great weekend! -Kevin Cosner
On Sat, 02 Jun 2007 11:44:08 -0700, kevin <medicalsou@hotmail.com> wrote: >Thank you for your help! I'm currently working on UNIX... that is why >my code is written as is...
What does using unix have to do with writing incorrect code that will invoke undefined behavior? >while( numLines<MAX_LINES && fgets( buff, MAX_LINE_LEN, fin )) >{ >> ++numLines; >> st[ numLines ] = strdup( buff ); >///The problem was def st[0] having trash... and fixed it by changing >the while loop as sugested...
st[0] did not have trash. It was initialized to a perfectly fine value, NULL. One problem was you chose to pass that value to the functions printf and strcmp in violation of their interfaces, thereby invoking undefined behavior. Another problem was that your while loop would attempt to store into the non-existent element st[MAX_LINES] (more undefined behavior) if you had MAX_LINES worth of data. Swapping the two statements inside the while loop will work on all systems, including unix. It also has the virtue of correcting both problems. You may think these are nits. They are also elementary (even fundamental) concepts of programming in c. If you don't come to understand them, similar problems will continue to haunt your code. Remove del for email
kevin wrote: > Thank you for your help! I'm currently working on UNIX... that is > why my code is written as is... > while( numLines<MAX_LINES && fgets( buff, MAX_LINE_LEN, fin )) > { >> ++numLines; >> st[ numLines ] = strdup( buff ); > ///The problem was def st[0] having trash... and fixed it by > changing the while loop as sugested...
You still haven't taken care of everything. fgets won't return a complete line when the line is longer than the buffer, and you have to worry about the final '\n'. Try ggets for the purpose, found at: <http://cbfalconer.home.att.net/download/> -- <http://www.cs.auckland.ac.nz/~pgut001/pubs/vista_cost.txt> <http://www.securityfocus.com/columnists/423> <http://www.aaxnet.com/editor/edit043.html> <http://kadaitcha.cx/vista/dogsbreakfast/index.html> cbfalconer at maineline dot net -- Posted via a free Usenet account from http://www.teranews.com
Barry Schwarz wrote: > On Sat, 02 Jun 2007 11:44:08 -0700, kevin <medicalsou @hotmail.com> > wrote: >> Thank you for your help! I'm currently working on UNIX... that is why >> my code is written as is... > What does using unix have to do with writing incorrect code that will > invoke undefined behavior? >> while( numLines<MAX_LINES && fgets( buff, MAX_LINE_LEN, fin )) >> { >>> ++numLines; >>> st[ numLines ] = strdup( buff ); >> ///The problem was def st[0] having trash... and fixed it by changing >> the while loop as sugested... > st[0] did not have trash. It was initialized to a perfectly fine > value, NULL. One problem was you chose to pass that value to the > functions printf and strcmp in violation of their interfaces, thereby > invoking undefined behavior. Another problem was that your while loop > would attempt to store into the non-existent element st[MAX_LINES] > (more undefined behavior) if you had MAX_LINES worth of data. Swapping > the two statements inside the while loop will work on all systems, > including unix. It also has the virtue of correcting both problems. > You may think these are nits. They are also elementary (even > fundamental) concepts of programming in c. If you don't come to > understand them, similar problems will continue to haunt your code.
Another name for "nits" is "details". And details are *very* important in any C program. ;-) -- +----------------------------------------------------------------+ | Charles and Francis Richmond richmond at plano dot net | +----------------------------------------------------------------+
|
 |
 |
 |
 |
|