Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

strncmp proper usage

Tags:

c

Here's the quick background: I've got a client and a server program that are communicating with each other over a Unix socket. When parsing the received messages on the server side, I am attempting to use strncmp to figure out what action to take.

The problem I'm having is figuring out exactly what to use for the length argument of strncmp. The reason this is being problematic is that some of my messages share a common prefix. For example, I have a message "getPrimary", which causes the server to respond with a primary server address, and a message "getPrimaryStatus", which causes the server to respond with the status of the primary server. My initial thought was to do the following:

if(strncmp(message,"getPrimary",strlen("getPrimary"))==0){
    return foo;
}
else if(strncmp(message,"getPrimaryStatus",strlen("getPrimaryStatus"))==0){
    return bar;
}

The problem with this is when I send the server "getPrimaryStatus", the code will always return foo because strncmp is not checking far enough in the string. I could pass in strlen(message) as the length argument to strncmp, but this seems to defeat the purpose of using strncmp, which is to prevent overflow in the case of unexpected input. I do have a static variable for the maximum message length I can read, but it seems like passing this in as the length is only making sure that if the message overflows, the effects are minimized.

I've come up with a few solutions, but they aren't very pretty, so I was wondering if there was a common way of dealing with this problem.

For reference, my current solutions are: Order my if / else if statements in such a way that that any messages with common prefixes are checked in order of descending length (which seems like a really good way to throw a landmine in my code for anyone trying to add something to it later on).

Group my messages with common prefixes together and look for the suffix first:

if(strncmp(message,"getPrimary",strlen("getPrimary"))==0){
    if(strncmp(message,"getPrimaryStatus",strlen("getPrimaryStatus"))==0){
        return bar;
    else
        return foo;
    }
}

But this just feels messy, especially since I have about 20 different possible messages that I'm handling.

Create an array of all the possible messages I have, add a function to my init sequence that will order the array by descending length, and have my code search through the elements of that list until it finds a match. This seems complicated and silly.

It seems like this should be a common enough issue that there ought to be a solution for it somewhere, but I haven't been able to find anything so far.

Thanks in advance for the help!

like image 596
Jordan Wills Avatar asked Dec 03 '10 02:12

Jordan Wills


3 Answers

Presuming that the string in message is supposed to be null-terminated, the only reason to use strncmp() here rather than strcmp() would be to be to prevent it looking beyond the end of message, in the case where message is not null-terminated.

As such, the n you pass to strncmp() should be the received size of message, which you ought to know (from the return value of the read() / recv() function that read the message).

like image 124
caf Avatar answered Nov 07 '22 22:11

caf


One technique is to compare the longest names first - order the tests (or the table containing the keywords) so that the longer names precede the shorter. However, taking your example:

GetPrimaryStatus
GetPrimary

You probably want to ensure that GetPrimaryIgnition is not recognized as GetPrimary. So you really need to compare using the length of the longer of the two strings - the message or the keyword.

Your data structure here might be:

static const struct
{
    char   *name;
    size_t  name_len;
    int     retval;
} Messages[] =
{
    { "getPrimaryStatus", sizeof("getPrimaryStatus"), CMD_PRIMARYSTATUS },
    { "getPrimary",       sizeof("getPrimary"),       CMD_PRIMARY       },
    ...
};

You can then loop through this table to find the relevant command. With some care you can limit the range that you have to look at. Note that the sizeof() values include the NUL at the end of the string. This is useful if you can null terminate your message

However, it is by far simplest if you can null terminate the command word in the message, either by copying the message somewhere or by modifying the message in situ. You then use strcmp() instead of strncmp(). A Shortest Unique Prefix lookup is harder to code.

One plausible way of finding the command word is with strcspn() - assuming your commands are all alphabetic or alphanumeric.

like image 36
Jonathan Leffler Avatar answered Nov 07 '22 23:11

Jonathan Leffler


I sense that you are using strncmp to prevent buffer overflow, however, the message is already copied into memory (i.e. the message buffer). Also, the prototype

int strncmp ( const char * str1, const char * str2, size_t num );

indicates that the function has no side effects (i.e. it does not change either input buffer) so there should be no risk that it will overwrite the buffer and change memory. (This is not the case for strcpy(). )

You could make sure that the length of your message buffer is longer than your longest command string. That way you are sure that you are always accessing memory that you own.

Also, if you insist on using strncmp you could store your list of commands in an array and sort it from largest to smallest. You could associate each string with a length (and possibly a function pointer to execute a handler).

Finally, you could find a C version of what C++ calls a map or what Ruby or PHP call associative arrays. This lets library handle this if-else tree for you efficiently and correctly.

like image 1
Commodore63 Avatar answered Nov 07 '22 22:11

Commodore63