Comparing strings in C - strcmp

Question

I'm having trouble comparing strings in C (with which I am fairly new to). I have socket on this server application waiting to accept data from a client. In this particular portion of my program I want to be able to execute a MySQL query based on the data received from the client. I want to be able to know when the received data has the value of "newuser" to initiate a simple registration procedure. Strcmp is returning a positive 1 value where I believe I should be getting a 0 because the values should be equal.

Source Code:

//setup socket
//loop and select structure to handle multiple connections

if ((nbytes = recv(i, buf, sizeof buf, 0)) <= 0) {
// got error or connection closed by client
    if (nbytes == 0) {
     // connection closed
     printf("selectserver: socket %d hung up\n", i);
    } else {
     perror("recv");
    }
    close(i); // bye!
    FD_CLR(i, &master); // remove from master set
} else {

    char check[] = "newuser";
    char fromUser[sizeof check];

    strncpy(fromUser,buf, sizeof check);
    printf("length of fromUser: %d\n", sizeof fromUser);
    printf("length of check: %d\n", sizeof check);
    printf("message from user: %s\n", fromUser);
    printf("check = %s \n", check);
    int diff = strcmp(fromUser, check);
    printf("compare fromUser to check: %d\n", diff);
    if ( strcmp(fromUser, check) == 0) {
        printf("aha! new user");
    }

Output:

length of fromUser: 8
length of check: 8
newuser from user: newuser
check = newuser 
compare fromUser to check:

I have a feeling I'm not handling the incoming buffer correctly or erroneously copying the buffer.

Answer 1

I believe the problem here (one of the problems here) is that fromUser (due to the way it is created) is not null terminated.

Answer 2

You miss '\0' char at the end of fromUser:

...
strncpy(fromUser,buf, sizeof check);
fromUser[strlen(check)] = '\0';

Answer 3

strncpy copies at most - in this case - sizeof check bytes. If the nul byte is not in that range it is not copied. You probably are getting the word "newuser" as part of a longer sentence, like "newuser blah blah" so you need to place that nul yourself

strncpy(fromUser, buf, sizeof check);
fromUser[sizeof check - 1] = '\0';

or use strlcpy, if available.

Answer 4

Replace with:

char check[] = "newuser\0";

Answer 5

Two changes required:

char fromUser[sizeof check] = {'\0'}; //Make all null characters
strncpy(fromUser,buf, sizeof check -1); //Last character is for null character.

Answer 6

This code seems off:

if ((nbytes = recv(i, buf, sizeof buf, 0)) <= 0) 
{
 // your stuff
} 
else {
const char *pCheck = "newuser";
char *fromUser = new char[nbytes];
strncpy(fromUser, buff, nbytes);
fromUser[nbytes] = '\0';
if(strcmp(fromUser,check)==0)
 // blah

delete [] fromUser;
}

Answer 7

Here's the sample code you gave in your question (with debugging code removed):

//setup socket
//loop and select structure to handle multiple connections

if ((nbytes = recv(i, buf, sizeof buf, 0)) <= 0) {
    [... exception handling here ...]
} else {
    char check[] = "newuser";
    char fromUser[sizeof check];

    strncpy(fromUser,buf, sizeof check);
    if ( strcmp(fromUser, check) == 0) {
        printf("aha! new user");
    }

This code is wrong; you're potentially copying more bytes from buf[] than have been received. This will lead to you comparing against garbage (that might by chance happen to match your "newuser" string). And as other people have said, you have a second bug due to not NUL terminating one of your strings.

In this case, I'd use memcmp(). This is like strcmp() but it takes a length parameter rather than expecting NUL-terminated strings.

//setup socket
//loop and select structure to handle multiple connections

if ((nbytes = recv(i, buf, sizeof buf, 0)) <= 0) {
    [... exception handling here ...]
} else {
    static const char check[] = "newuser";
    const size_t check_len = sizeof(check) - 1; // exclude the NUL terminator
    if (nbytes >= check_len && memcmp(buf, check, check_len) == 0) {
        printf("aha! new user");
    }

P.S. Not directly related, but recv() can fail by returning -1 with errno==EINTR. This isn't an error condition, you just need to try again. Normally this happens so rarely that people get away without checking it, until they integrate with some other code that uses signals and suddenly their code randomly fails.

In a select()-based app, you should also be setting your sockets to non-blocking and then check for errno==EAGAIN, and go back to the select() in that case. This can happen if the TCP/IP stack receives a corrupted packet - it thinks it has a packet so select() will tell you it's readable, it's only when you try to read it that the TCP/IP stack does the checksum calculation and realises it has to throw away the data. It'll then either block (bad), or if it's set to nonblocking then it will return -1 with errno==EAGAIN.