Segmentation fault while processing argv

Question

This procedure should convert a string that contains a set of double numbers separated by comma (e.g. 7.2,9.5,-5.515) to a vector of double type.

  void ToDoubleVec(int d,const char* commaSeparated,double *result)
    {
        int i;      
        result[0]=atof(strtok(commaSeparated,","));
        for(i=1;i<d;i++)
            result[i]=atof(strtok(NULL,","));   
    }

Here is the snippet of program that calls it:

#include <stdio.h>
#include <stdlib.h>
#include <time.h>

int main(int argc,char** argv)
    {
    ...
        int i,dim=atoi(argv[1]);
        double *lower;
        lower = malloc(dim*sizeof(double));
        ToDoubleVec(dim,argv[2],lower);
    ...
    }

Debugger's output:

40      lower = malloc(dim*sizeof(double)); 
(gdb) s
42      ToDoubleVec(dim,argv[2],lower);
(gdb) s
ToDoubleVec (d=2, commaSeparated=0x7fffffffe9d3 "2.3,-62.1", result=0x603010) at testPSO.c:11
11      result[0]=atof(strtok(commaSeparated,","));
(gdb) s

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff77f56bb in ?? () from /lib/x86_64-linux-gnu/libc.so.6

Why doesn't it work? I was sure that I've allocated enough memory for the array and also parameters seems to be passed correctly.

Answer 1

You can reduce your code to this SSCCE (Short, Self-Contained, Correct Example), which crashes nicely when you leave out #include <string.h> and does not compile cleanly when you add #include <string.h>:

segv.c: In function ‘ToDoubleVec’:
segv.c:8:5: warning: implicit declaration of function ‘strtok’ [-Wimplicit-function-declaration]
segv.c:8:20: warning: initialization makes pointer from integer without a cast [enabled by default]
segv.c:14:20: warning: assignment makes pointer from integer without a cast [enabled by default]

Code:

#include <stdlib.h>
//#include <string.h>

static void ToDoubleVec(int d, const char* commaSeparated, double *result)
{
    int i;      
    result[0] = atof(strtok(commaSeparated, ","));
    for (i = 1; i < d; i++)
        result[i] = atof(strtok(NULL, ","));   
}

int main(void)
{
    int dim = 2;
    double *lower = malloc(dim*sizeof(double));
    char arg[] = "7.2,9.5,-5.515";
    ToDoubleVec(dim, arg, lower);
}

Passing the return value from a function such as strtok() which can return a null pointer directly to a function such as atof() which does not tolerate null pointers is foolhardy; it leads to crashes. If everything is correct, you'll be OK; if not, you'll crash and burn.

The unchecked memory allocation is a similar problem; you didn't even check that dim was non-zero (and non-negative) before doing the memory allocation in the original.

#include <assert.h>
#include <string.h>
#include <stdlib.h>

static void ToDoubleVec(int d, char *commaSeparated, double *result)
{
    int i;      
    char *number = strtok(commaSeparated, ",");
    if (number != 0)
    {
        result[0] = atof(number);
        for (i = 1; i < d; i++)
        {
            number = strtok(NULL, ",");
            if (number != 0)
                result[i] = atof(number);
        }
    }
}

int main(void)
{
    int dim = 2;
    double *lower = malloc(dim*sizeof(double));
    char arg[] = "7.2,9.5,-5.515";
    assert(lower != 0);
    ToDoubleVec(dim, arg, lower);
}

You could — and in one version of the code I did — add error printing to report if the tests on number failed. But the crash is caused by the implicit declaration of strtok() as returning int and not char *.