0

I've been working on a c program called ft_split_whitespaces.

What the program is supposed to do, is to find words and put them into a string array.

I have to do this for a project and I can't use any function except malloc() and sizeof(). I'm only using printf() and other stuff for debugging purposes.

So, I ran into a problem that when I would debug the program it would work fine and if I would run it I would get segfault.

I researched this topic and it turned out to be a heisenbug. I tried several ways to find the bug by turning off the debugger optimization but the result is always that if I go over the program step by step it works and otherwise, it won't.

Another weird thing I ran into that the program would work perfectly on my computer but when running on a different computer it would ones again segfault.

So here is my program and any help would be great.

#include <stdlib.h>
#include <stdio.h>
#define W1(c)   (c == ' ' ||  c == '\n' || c == '\t')
#define W2(c)   (c == '\v' || c == '\f' || c == '\r')

int     ft_wordcount(char *str)
{
    int i;
    int ws;

    ws = 1;
    i = 0;
    while (*str)
    {
        if (W1(*str) || W2(*str))
            ws = 1;
        else
        {
            if (ws)
                i++;
            ws = 0;
        }
        str++;
    }
    return (i);
}

int     ft_wordlen(char *str)
{
    int ln;

    ln = 0;
    while (!(W1(*str)) && !(W2(*str)) && *str != '\0')
    {
        ln++;
        str++;
    }
    return (ln);
}

char    **ft_assign(char *str)
{
    int     i;
    char    **res;
    int wordcount;
    int ln;

    i = 0;
    wordcount = ft_wordcount(str);
    res = (char **)malloc(wordcount * sizeof(char));
    while (i < wordcount)
    {
        if (!(W1(*str)) && !(W2(*str)))
        {
            ln = ft_wordlen(str);
            res[i] = malloc((ln + 1) * sizeof(char));
            res[i][ln] = '\0';
            str += ln;
            i++;
        }
        str++;
    }
    return (res);
}

char **ft_split_whitespaces(char *str)
{
    char    **res;
    int     i;
    int     wordcount;
    int     pos;

    wordcount = ft_wordcount(str);
    i = 0;
    res = ft_assign(str);
    while (i < wordcount)
    {
        if (!(W1(*str)) && !(W2(*str)))
        {
            pos = 0;
            while (!(W1(*str)) && !(W2(*str)) && *str != '\0')
                res[i][pos++] = *(str++);
            i++;
        }
        str++;
    }
    return (res);
}


int main(void)
{
    int i;
    int ln;
    i = 0;
    char test[] = "yes  no yuuh     WORD   adjdsfjlksdj   sdfjkdsfjkjsd     sfdkjsdlkjf  sfdds\tfsd";
    char **words;
    words = ft_split_whitespaces(test);
    ln = ft_wordcount(test);
    while (i < ln)
    {
        printf("%s\n", words[i]);
        i++;
    }
}
ivan_pozdeev
  • 33,874
  • 19
  • 107
  • 152
lsoeth
  • 55
  • 1
  • 4
  • 2
    You never need `* sizeof(char)`, that's always 1. Consider checking `*str != '\0'` before checking for whitespace. Look carefully at `res = (char **)malloc(wordcount * sizeof(char));` Are you allocating characters or pointers to characters? They are not the same size. – Retired Ninja Jul 24 '18 at 04:32
  • can we update the title to be more specific, as we get more info on the problem and solution? TIA. – benc Jul 24 '18 at 05:05
  • As @Retired said, you are allocating a char instead of char* – Amichai Jul 24 '18 at 05:17
  • @RetiredNinja Wow that really fixed it! Thanks so much, looking at this now is pretty embarrassing. I have two followup questions tho, why did it work with a debugger and is there a system where sizeof(char) != 1? – lsoeth Jul 24 '18 at 06:12
  • You accessed illegal memory locations by not allocating enough memory there. That invokes Undefined Behavior which means that anything could happen. In your case, it unfortunately happened to "work". `sizeof(char)` is mandated to be 1 by the C standard. So you can safely omit it, although its perfectly fine if you keep it there for brevity. I don't think there are systems where `sizeof(char) != 1`. If there were, then that implementation is not standard conforming. – Spikatrix Jul 24 '18 at 07:18
  • Note that `sizeof` is an operator like `+` is an operator; it is not a function. – Jonathan Leffler Jul 24 '18 at 07:39

2 Answers2

3

There are multiple problems in your code:

  • The argument in macros W1 and W2 is not properly parenthesized in the expansion. You should at least write this:

    #define W1(c)   ((c) == ' '  || (c) == '\n' || (c) == '\t')
    #define W2(c)   ((c) == '\v' || (c) == '\f' || (c) == '\r')
    

    but c would still be multiply evaluated, so side effects will occur multiple times. Don't use macros for this, use inline functions:

    static inline int W1(int c) { return (c == ' '  ||  c == '\n' || c == '\t'); }
    static inline int W2(int c) { return (c == '\v' || c == '\f' || c == '\r'); }
    
  • The allocation sizes are incorrect in ft_assign: res = (char **)malloc(wordcount * sizeof(char)); should be written:

    res = malloc(wordcount * sizeof(char *));
    

    or better:

    res = malloc(wordcount * sizeof(*res));
    
  • You forget to copy the string into the allocated block: res[i][ln] = '\0'; should be preceded by:

    memcpy(res[i], str, ln);
    

    Or if strndup is available on your system, simply replaced with

    res[i] = strndup(str, ln);
    
chqrlie
  • 131,814
  • 10
  • 121
  • 189
1

The main problem of your code is the res allocation, in ft_assign:

res = (char **)malloc(wordcount * sizeof(char));

Should be:

res = (char **)malloc(wordcount * sizeof(char*));

Or better:

res = malloc(wordcount * sizeof *res);

Indeed, sizeof(char) is one, sizeof(char*) is 4 (on 32bits cpu) or 8 (on 64bits), So by missing this simple *, you allocate too few memory.


See also: why you should not cast malloc result

Mathieu
  • 8,840
  • 7
  • 32
  • 45