Segmentation fault when converting char * to char **-CodePudding

I'm trying to split a sentence (char *) to an array of words (char **). The problem is that my function that does just that sometimes doesn't return a valid char **.

char **get_words(char *buffer, char delimiter)
{
    char **words = malloc(sizeof(char *) * 4096);
    for (int i = 0; i < 4096; i  )
        words[i] = malloc(sizeof(char) * 4096);
    int word_count = 0;
    int l = 0;
    for (int i = 0; buffer[i] != '\0' && buffer[i]  != '\n'; i  , l  ) {
        if (buffer[i] == delimiter) {
            words[word_count][l] = '\0';
            word_count  ;
            l = -1;
        }
        else
            words[word_count][l] = buffer[i];
    }
    words[word_count][l] = '\0';
    return (words);
}

I first use it like this:

char *buffer = malloc(sizeof(char) * 50);
buffer = "/login test\n";
char **words = get_words(buffer, ' ');
printf("Words[0] = %s", words[0]);

And it works fine.

However when I do it the same way with this:

char **reply = get_words("502 Command doesn't exist.\n", ' ')

I can't even print reply[0][0] (see below) without having a segmentation fault. Moreover, I tried to debug this using valgrind but when I use it the program doesn't crash and everything works so I can't find what's wrong.

printf("Reply[0][0] = %d\n", reply[0][0]);

printf("Reply[0][0] = %c\n", reply[0][0]);

EDIT: Here is a reproductible example.

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <fcntl.h>
#include <assert.h>

char **get_words(char *buffer, char delimiter)
{
    printf("buffer = %s\n", buffer);
    char **words = malloc(sizeof(char *) * 100);
    if (words == NULL) {
        printf("Malloc Error\n");
        exit(84);
    }
    for (int i = 0; i < 100; i  ) {
        words[i] = malloc(sizeof(char) * 100);
        if (words[i] == NULL) {
            printf("Malloc Error\n");
            exit(84);
        }
    }
    int word_count = 0;
    int l = 0;
    for (int i = 0; buffer[i] != '\0' && buffer[i]  != '\n'; i  , l  ) {
        if (buffer[i] == delimiter) {
            words[word_count][l] = '\0';
            word_count  ;
            l = -1;
        }
        else
            words[word_count][l] = buffer[i];
    }
    words[word_count][l] = '\0';
    return (words);
}

int main()
{
    char *buffer = malloc(sizeof(char) * 100);
    buffer = "hello world !\n";
    char **words = get_words(buffer, ' ');
    printf("words[0]= %s\n", words[0]);
    free (buffer);
    char **reply = get_words("Second call\n", ' ');
    printf("reply[0] = %s\n", reply[0]);
}

CodePudding user response：

If you need help in learning programming, you can try a static analyzer. This is a program that performs code reviews and finds suspicious code fragments. Static analyzers can't replace code reviews performed by a teammate. However, analyzers complement code reviews and help find many errors at earliest stages.

Let's run the online version of the PVS-Studio analyzer for the code sample attached to the question. The first interesting and important warning is the following warning: V1031 The malloc function is not declared. Passing data to or from this function can be affected.

Without declaring the malloc function, the program runs in a strange way. According to the C language, if a function is not declared, it returns int. But actually, it's a pointer. You can find out why this is dangerous here. Let's fix this problem by adding #include <stdlib.h>.

Now the analyzer issues another warning — we get a more serious issue: 43:1: note: V773 The 'buffer' pointer was assigned values twice without releasing the memory. A memory leak is possible.

The issue is in the following code fragment:

char *buffer = malloc(sizeof(char) * 100);
buffer = "hello world !\n";
....
free (buffer);

The pointer value is overwritten. To copy a string to the buffer, a programmer should use special functions, for example strcpy. Let's fix this.

Here's the fixed code.

#include <unistd.h>
#include <stdio.h>
#include <string.h>
#include <fcntl.h>
#include <assert.h>
#include <stdlib.h>

char **get_words(char *buffer, char delimiter)
{
    printf("buffer = %s\n", buffer);
    char **words = malloc(sizeof(char *) * 100);
    if (words == NULL) {
        printf("Malloc Error\n");
        exit(84);
    }
    for (int i = 0; i < 100; i  ) {
        words[i] = malloc(sizeof(char) * 100);
        if (words[i] == NULL) {
            printf("Malloc Error\n");
            exit(84);
        }
    }
    int word_count = 0;
    int l = 0;
    for (int i = 0; buffer[i] != '\0' && buffer[i]  != '\n'; i  , l  ) {
        if (buffer[i] == delimiter) {
            words[word_count][l] = '\0';
            word_count  ;
            l = -1;
        }
        else
            words[word_count][l] = buffer[i];
    }
    words[word_count][l] = '\0';
    return (words);
}

int main()
{
    char *buffer = malloc(sizeof(char) * 100);
    if (buffer == NULL)
        exit(84);
    strcpy(buffer, "hello world !\n");
    char **words = get_words(buffer, ' ');
    printf("words[0]= %s\n", words[0]);
    free (buffer);
    char **reply = get_words("Second call\n", ' ');
    printf("reply[0] = %s\n", reply[0]);
}

I can't say that this code is perfect and secure, but it runs. So, using static analyzers to find errors, you can improve your learning process.