Why segfault and no NULL match in for loop?

[somebody]

There are two files below named search.c and search.h.
In the for loop in search.c, the for loop never exits,
even if mystruct.field1 has no match. Instead of
exiting the for loop it keeps going until it segfaults.
This seems to be related to the strcmp with the NULL
value. There are 2 comments below that indicate
the segfaults. I guess the question is, when there
is no match, how to I detect that and return without
a segfault?

-Thanks


SEARCH.C ----------------------------------------

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

#include "search.h"


int search(char * field1)
{
int i = 0;


/* This is never = to NULL, even when no match for field1 */

for(i=0; mystruct.field1 != NULL; i++) {

if(strcmp(field1, mystruct.field1) == 0) {
return(mystruct.numfield);
}


/*THIS SEGFAULTS ALWAYS!!!!*/
/*if (strcmp(mystruct.field1, NULL) == 0) {*/
/*}*/

}

return(0);
}



int main(void)
{
int retval = 0;

/* THIS CAN CAUSE SEGFAULT TOO!!! */
/*This works if you match, for example, CCC. But if*/
/*you change the CCC to say YYY, which is not in the*/
/*struct, it segfaults.*/

char str[] = "CCC";

retval = search(str);

printf("retval = %d\n", retval);

return 0;
}


SEARCH.H -----------------------------


struct _mystruct
{
char field1[4];
char field2[4];
char field3[2];
char field4[3];
int numfield;
};


struct _mystruct mystruct[] =
{
"AAA", "EEE", "R", "DF", 25,
"BBB", "FFF", "R", "DF", 25,
"CCC", "GGG", "R", "DF", 99,
"DDD", "HHH", "R", "DF", 13,
};

 

[Walter Roberson]

There are two files below named search.c and search.h.
In the for loop in search.c, the for loop never exits,
even if mystruct.field1 has no match. Instead of
exiting the for loop it keeps going until it segfaults.
This seems to be related to the strcmp with the NULL
value.

int search(char * field1)
{
int i = 0;

/* This is never = to NULL, even when no match for field1 */

for(i=0; mystruct.field1 != NULL; i++) {

If you examine your mystruct data, you will see that you do not
have any entries in which mystruct[<anything>] == NULL .
You therefore iterate off the end of mystruct[], and start
testing a non-existant mystruct.field1 entry.

if(strcmp(field1, mystruct.field1) == 0) {
return(mystruct.numfield);
}

}
return(0);
}

struct _mystruct
{
char field1[4];
char field2[4];
char field3[2];
char field4[3];
int numfield;
};


struct _mystruct mystruct[] =
{
"AAA", "EEE", "R", "DF", 25,
"BBB", "FFF", "R", "DF", 25,
"CCC", "GGG", "R", "DF", 99,
"DDD", "HHH", "R", "DF", 13,
};

 

[Richard Tobin]

/* This is never = to NULL, even when no match for field1 */

for(i=0; mystruct.field1 != NULL; i++) {

[/QUOTE]

If you examine your mystruct data, you will see that you do not
have any entries in which mystruct[<anything>] == NULL .
You therefore iterate off the end of mystruct[], and start
testing a non-existant mystruct.field1 entry.

And since mystruct is an array, you can just loop over its known
length:

for(i=0; i<sizeof(mystruct)/sizeof(mystruct[0]); i++)

Don't use names starting with underscore, they're reserved.

-- Richard

 

[Default User]

There are two files below named search.c and search.h.
In the for loop in search.c, the for loop never exits,
even if mystruct.field1 has no match. Instead of
exiting the for loop it keeps going until it segfaults.
This seems to be related to the strcmp with the NULL
value. There are 2 comments below that indicate
the segfaults. I guess the question is, when there
is no match, how to I detect that and return without
a segfault?

-Thanks


SEARCH.C ----------------------------------------

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

#include "search.h"


int search(char * field1)
{
int i = 0;


/* This is never = to NULL, even when no match for field1 */

for(i=0; mystruct.field1 != NULL; i++) {

When will your conditional ever be true? Those character arrays can
NEVER be NULL. You need to add another "blank" array entry, and test
for an empty string or something.

for(i=0; mystruct.field1[0] != '\0'; i++) {

if(strcmp(field1, mystruct.field1) == 0) {
return(mystruct.numfield);
}


/*THIS SEGFAULTS ALWAYS!!!!*/
/*if (strcmp(mystruct.field1, NULL) == 0) {*/
/*}*/

Well, yeah. NULL is not a string, and passing a null pointer to most of
the string-handling routines causes undefined behavior. To test for a
null pointer (which you can't have), use equality.

struct _mystruct
{
char field1[4];
char field2[4];
char field3[2];
char field4[3];
int numfield;
};

Try adding:

struct _mystruct mystruct[] =
{
"AAA", "EEE", "R", "DF", 25,
"BBB", "FFF", "R", "DF", 25,
"CCC", "GGG", "R", "DF", 99,
"DDD", "HHH", "R", "DF", 13, "", "", "", "", 0
};

Brian

 

[Default User]

Richard Tobin wrote:

And since mystruct is an array, you can just loop over its known
length:

for(i=0; i<sizeof(mystruct)/sizeof(mystruct[0]); i++)

True, but only because he has global data. As soon as he moves that
array into main() or another function and passes it into search() as he
should, he'll be right back where he started. He could add a size
parameter, of course, or use a sentinel value like he tried and failed
to do.





Brian

 

[Keith Thompson]

NEVER define an actual object in a .h file. Doing so will cause it
to be multiply defined, and result in link or run time errors. Add
the word "extrn" to the definition, and delete the initialization.
Put the above structure in ONLY one of the files in which you
#include "search.h".

It's "extern", not "extrn".

 

[jaysome]

There are two files below named search.c and search.h. In the for
loop in search.c, the for loop never exits, even if
mystruct.field1 has no match. Instead of exiting the for loop
it keeps going until it segfaults. This seems to be related to the
strcmp with the NULL value. There are 2 comments below that
indicate the segfaults. I guess the question is, when there is no
match, how to I detect that and return without a segfault?

... snip to content of search.h ...

struct _mystruct mystruct[] =
{
"AAA", "EEE", "R", "DF", 25,
"BBB", "FFF", "R", "DF", 25,
"CCC", "GGG", "R", "DF", 99,
"DDD", "HHH", "R", "DF", 13,
};

NEVER define an actual object in a .h file. Doing so will cause it
to be multiply defined, and result in link or run time errors. Add
the word "extrn" to the definition, and delete the initialization.
Put the above structure in ONLY one of the files in which you
#include "search.h".

In addition to the correction noted by Keith ("extrn" should be
"extern"), it should be noted that defining an actual object in a .h
file does not necessarily mean that it will be multiply defined and
result in a link or run time error.

If the header file is included by a single translation unit, there
will be no link or run time errors. You only run into trouble when
multiple translation units include said header file and *think* they
are accessing the same data (something I have witnessed in practice).

Why would you ever want to define an actual object in a .h file? I can
think of only one instance, which is if the .h file is shared between
two or more programs (and included by a single translation unit in
each, of course). I've used this technique, but only sparingly. Think
of gcc and gcov using the same objects with file scope internal
linkage, as an example (I'm not saying they do this, just that it is
an acceptable option, IMHO, especially if there are a lot of the same
objects used in both).

(My actual usage of the above is almost the same code running as
programs/processes on two dissimilar processors, which was one of the
requirements necessary to achieve a certain level of system safety
assurance).

NEVER define an actual object in a .h file, unless you really know
what you're doing.

A .h file is NOT a place for headers, it is a place to define the
data you want to export from a .c file.

Saying a ".h file is NOT a place for headers" is like saying a "house
is not a place for a home". I think you meant "objects" instead of
"headers".

Best regards

 

[Keith Thompson]

No, I wrote what I meant. I mean you don't just collect all the
headers and export them to a .h file. You only export those that
are needed elsewhere.

I think the point is that a .h file (almost always) *is* a "header".
Are you using the term "header" in some other sense?