Why segfault and no NULL match in for loop?

Discussion in 'C Programming' started by somebody, May 2, 2007.

  1. somebody

    somebody Guest

    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,
    };
    somebody, May 2, 2007
    #1
    1. Advertising

  2. In article <>,
    somebody <> wrote:
    >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,
    >};



    --
    Programming is what happens while you're busy making other plans.
    Walter Roberson, May 2, 2007
    #2
    1. Advertising

  3. In article <f1b4ns$6lt$>,
    Walter Roberson <-cnrc.gc.ca> wrote:

    >> /* 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.


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

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

    >>struct _mystruct


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

    -- Richard
    --
    "Consideration shall be given to the need for as many as 32 characters
    in some alphabets" - X3.4, 1963.
    Richard Tobin, May 2, 2007
    #3
  4. somebody

    Default User Guest

    somebody wrote:

    > 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, May 3, 2007
    #4
  5. somebody

    Default User Guest

    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
    Default User, May 3, 2007
    #5
  6. CBFalconer <> writes:
    [...]
    > 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".

    --
    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, May 3, 2007
    #6
  7. somebody

    jaysome Guest

    On Wed, 02 May 2007 20:14:34 -0400, CBFalconer <>
    wrote:

    >somebody wrote:
    >>
    >> 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
    --
    jay
    jaysome, May 3, 2007
    #7
  8. CBFalconer <> writes:
    > jaysome wrote:
    >> CBFalconer <> wrote:
    >>

    > ... snip ...
    >>
    >>> 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".

    >
    > 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?

    --
    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, May 3, 2007
    #8
    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. Replies:
    5
    Views:
    26,481
    Mike Schilling
    Mar 29, 2006
  2. Mr. SweatyFinger

    why why why why why

    Mr. SweatyFinger, Nov 28, 2006, in forum: ASP .Net
    Replies:
    4
    Views:
    855
    Mark Rae
    Dec 21, 2006
  3. Mr. SweatyFinger
    Replies:
    2
    Views:
    1,744
    Smokey Grindel
    Dec 2, 2006
  4. Andrey Vul
    Replies:
    8
    Views:
    670
    Richard Bos
    Jul 30, 2010
  5. Isaac Won
    Replies:
    9
    Views:
    349
    Ulrich Eckhardt
    Mar 4, 2013
Loading...

Share This Page