comparing using strcmp

Question

compiling with gcc C99

I am trying to compare 2 string using string compare. However, I seem to be getting a stack dump on the strcmp line.

**attribute will contain these, so I am looking for frametype.

[name] [time] [type] [time]
[name] [callref] [type] [string]
[name] [port] [type] [int16]
[name] [frametype] [type] [int16]

Is this correct way to compare.

Many thanks for any suggestions,

void g_start_element(void *data, const char *element, const char **attribute)
{
    for(i = 0; attribute[i]; i++)
    {
    /* Only interested in the frametype */
        if(strcmp(attribute[i], "frametype") == 0)
        {
            /* do some work here */
        }

     }
}

Answer 1

The context of this code is expat parsing - see this post. The attributes array is alternating name and value, with a single 0 to terminate.

Unless you're looking for any attribute whose name or value is equal to your test string (which would be somewhat unusual), then your code should increment i by 2 rather than 1 - so that it steps over both a name and a value.

You should be comparing either attribute[i] to match a name, or or attribute[i+1] to match a value.

Don't assume that the attributes will be in any particular order. Currently you're only looking at attribute1, which is the value of the first attribute returned. If there are more than one attribute, they can be returned in any order.

Answer 2

You'll need to have a null string in order to terminate the for loop:

[name] [time] [type] [time]
[name] [callref] [type] [string]
[name] [port] [type] [int16]
[name] [frametype] [type] [int16]
null

Without this the for loop won't terminate and you'll end up with attribute[i] pointing to garbage when you call strcmp.

Answer 3

1. How is the attribute array initialized? A NULL element may have slipped in.
2. Also, the array element must end with a NULL.
3. You may consider using strncmp() as a safer alternative to strcmp().

Answer 4

Is your input array (attribute) NULL-terminated? You don't list it as being, but the code requires it otherwise it will just walk along into random memory, which is a good cause of segfaults.

Insert a print of the index and/or the attribute being compared, to see if it behaves the way you expect with respect to termination.

Not so sure about the string-comparison part of the question ... If the input contains those square brackets and so on, you won't ever find anything since you are looking at the start of each string. Try strstr() if that is the case, it finds substrings.

Answer 5

Add logging and dump all the attributes and the indexer value along the way. This will help identify what's going wrong.

Answer 6

Beyond not being null terminated, why not use strncmp() ? :)

Answer 7

I would add a check for NULL so you have

if(attribute[1] != NULL && strcmp("frametype", attribute[1]) == 0)

So you are not dereferencing a NULL pointer.