strcmp problems.... ??? Segmentation fault

[kevin]

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, 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);
if( strcmp(st, s) == 0 )
return 1;
}

return 0;
}

 

[Johan Bengtsson]

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);
if( strcmp(st, 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.

 

[Barry Schwarz]

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, 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);

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, s) == 0 )

More undefined behavior for the same reason.

return 1;
}

return 0;
}

Remove del for email

 

[kevin]

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

 

[Barry Schwarz]

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

 

[Charles Richmond]

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