Segmentation fault on sprintf in C

Question

This function is in a socket server. When the client sends a query, the server takes the query and finds matches from a linked list. The function works fine for the first few queries, and then a segmentation fault occurs. The problem occurs at the sprintf call(the one after "Before sprintf.\n"). I really don't understand why it works just for a few times. What have I done wrong?

char* searchNode(char* query) {
    int i, isFound, count = 0;
    node* temp = head;
    char* searchResult = calloc(1, sizeof(* searchResult));
    char* finalResult = calloc(1, sizeof(* finalResult));;

    printf("Before search node.\n");

    while(temp->next) {
        isFound = TRUE;
        temp = temp->next;
        for(i = 0; i < strlen(query); i++) { /* compare each char in both strings */
            if(tolower(query[i]) != tolower(temp->foodName[i])) {
                isFound = FALSE;
                break;
            }
        }
        if(isFound == TRUE) { /* if a match is found, write it into the temp string */
        printf("Match found.\n");
            searchResult = realloc(searchResult, strlen(searchResult) + 1 + strlen(nodeToString(temp)) + 1);
        printf("Before sprintf.\n");
            sprintf(searchResult, "%s%s", searchResult, nodeToString(temp));
            count++; /* count the number of results found */
        }
    }

    printf("Before finalise string.\n");

    if(count > 0) { /* if at least a result is found, add combine all results with a head line*/
        sprintf(finalResult, "%d food item(s) found.\n\n", count);
        strcat(finalResult, searchResult);
        free(searchResult);
        return finalResult;
    }

    /* if no match is found, return this message */
    return "No food item found.\nPlease check your spelling and try again.\n";
}

Answer 1

You forgot to test the success of  calloc. And you are using it incorrectly: you need to allocate enough bytes for the 0-terminated string in it.

Notice that char* searchResult = calloc(1, sizeof(* searchResult)); is deeply wrong: it is equivalent to /* wrong code*/ char* searchResult= calloc(1,1); and you cannot do that (you need to allocate a wide enough string); you've got some undefined behavior and you have the bad luck that it does not crash (SO contains a lot of answers about UB, see e.g. this one).

You should use snprintf(3) (perhaps with strdup(3)) and you should take into account the result of snprintf + 1 for the terminating zero byte. You may want to use asprintf(3) if your system provides it.

Please compile with all warnings and debug info gcc -Wall -Wextra -g. Use valgrind and the gdb debugger.

Answer 2

I don't know what that sprintf is going to do when passed searchResult as an argument. The man page on my system suggests that it is undefined:

C99 and POSIX.1-2001 specify that the results are undefined if a call to sprintf(), snprintf(), vsprintf(), or vsnprintf() would cause copying to take place between objects that overlap (e.g., if the target string array and one of the supplied input arguments refer to the same buffer).

You should probably be using strcat there instead.

Answer 3

just read the failing line

 sprintf(searchResult, "%s%s", searchResult, nodeToString(temp));

It says print searchResult and other stuff into searchResult. It cant possibly work

searchResult is not the Tardis

Answer 4

You are adjusting what searchResult points to, but finalResult stays pointing to 1 char only, although it finally gets copied all the search result into, which most propably is more the 1 char, with this writing to invalid memory and provoking undefined behaviour.