inexplicable segfaults

[KBriggs]

I am writing a simple file/text parser to read a config file for some code I am working on. It's dead simple and not particularly smart but it should get the job done. The code reads a config file:

readlength=2500000
start=0
finish=25000000

cutoff=20000
samplingfreq=250000
poles=10
filterpadding=500

threshold=0.5
hysteresis=0.4

numbins=20

event_minpoints=100
event_maxpoints=10000
subevent_minpoints=50

into a config struct:

struct Configuration
{
    char filepath[1024]; //input file

    //file reading parameters
    long long int start;
    long long int finish;
    long long int readlength;

    //filter parameters
    double cutoff;
    long int poles; //must be even
    long int padding;

    //detection parameters
    double threshold;
    double hysteresis;

    long int numbins;

    long int event_minpoints;
    long int event_maxpoints;
    long int subevent_minpoints;


};
typedef struct Configuration configuration;

Using this function:

void read_config(configuration *config)
{
    char *configline;
    char *name;
    char *value;
    char *newline;
    long int cutoff = 0;
    long int samplingfreq = 0;
    if ((configline = malloc(STRLENGTH*sizeof(char)))==NULL)
    {
        printf("Cannot allocate config line string\n");
        abort();
    }
    int test;
    FILE *configfile;
    if ((configfile = fopen("config.dat","r"))==NULL)
    {
        printf("Cannot find config file: \"config.dat\"!");
        abort();
    }


    while ((configline = fgets(configline, STRLENGTH, configfile)) != NULL)
    {
        name = strtok(configline,"=");
        value = strtok(NULL,"=\n");
        if (strcmp(name,"readlength") == 0)
        {
            config->readlength = strtoll(value,NULL,10);
        }
        else if (strcmp(name,"start") == 0)
        {
            config->start = strtoll(value,NULL,10);
        }
        else if (strcmp(name,"finish") == 0)
        {
            config->finish = strtoll(value,NULL,10);
        }
        else if (strcmp(name,"cutoff") == 0)
        {
            cutoff = strtol(value,NULL,10);
        }
        else if (strcmp(name,"samplingfreq") == 0)
        {
            samplingfreq = strtol(value,NULL,10);
        }
        else if (strcmp(name,"poles") == 0)
        {
            config->poles = strtol(value,NULL,10);
        }
        else if (strcmp(name,"filterpadding") == 0)
        {
            config->padding = strtol(value,NULL,10);
        }
        else if (strcmp(name,"threshold") == 0)
        {
            config->threshold = strtod(value,NULL);
        }
        else if (strcmp(name,"hysteresis") == 0)
        {
            config->hysteresis = strtod(value,NULL);
        }
        else if (strcmp(name,"numbins") == 0)
        {
            config->numbins = strtol(value,NULL,10);
        }
        else if (strcmp(name,"event_minpoints") == 0)
        {
            config->event_minpoints = strtol(value,NULL,10);
        }
        else if (strcmp(name,"event_maxpoints") == 0)
        {
            config->event_maxpoints = strtol(value,NULL,10);
        }
        else if (strcmp(name,"subevent_minpoints") == 0)
        {
            config->subevent_minpoints = strtol(value,NULL,10);
        }
    }
    config->cutoff = 2.0 *(double) cutoff/(double) samplingfreq;
    free(configline);
}

Here is where it gets wierd. You'll notice that there is an unused variable (filepath) in the config struct. This variable is not referenced or used anywhere in the code, ever. Yet if I comment out the declaration of char filepath[1024], the code segfaults partway through the read_config() function.

My best guess is that there is a buffer overflow elsewhere and it just so happens that the memory allocated for filepath happened to be there to catch it up until now, but I can't work out where it might be happening. With the declaration commented out, the read_config() function gets as far as reading the "padding" variable before it crashes. Yet when the declaration is there, then all the variabled are read correctly and everything seems to work. Obviously there is something broken, however. Can anyone shed some light on what is actually happening?

[std10093]

With just a glance:

config->cutoff = 2.0 *(double) cutoff/(double) samplingfreq;

division by zero?

I do not think so however, just a "random" guess.

[KBriggs]

The crash happens well before that line, in the while loop over fgets just after it reads the filterpadding variable. But no, there's no division by zero - in the case where I leave filepath declared, samplingfreq is 250000 after that loop finishes. I can also set samplingfreq = 1 at the declaration with the same result.

[jimblumberg]

Did you run your program with your debugger? The debugger should be able to tell you exactly where the problem is detected and allow you to view the variables at the time of the crash.


Jim

[anduril462]

Also, try using valgrind or electric fence to help you find memory problems.

There is nothing wrong with the code you posted. It is almost certainly elsewhere, in that vast collection of code you didn't post.

[KBriggs]

I was hoping to not have to figure out how to use the code::blocks debugger ^_^. But I might have to. I am used to coding in Linux and now I have to do it in windows at work, so all my tools are unfamiliar to me suddenly.

If anyone is interested in trying this at home, here is a minimal working example:

#define STRLENGTH 1024
#include<stdio.h>
#include<stdlib.h>
#include<string.h>

struct Configuration
{
    char filepath[1024]; //input file

    //file reading parameters
    long long int start;
    long long int finish;
    long long int readlength;

    //filter parameters
    double cutoff;
    long int poles; //must be even
    long int padding;

    //detection parameters
    double threshold;
    double hysteresis;

    long int numbins;

    long int event_minpoints;
    long int event_maxpoints;
    long int subevent_minpoints;


};
typedef struct Configuration configuration;




void read_config(configuration *config)
{
    char *configline;
    char *name;
    char *value;
    char *newline;
    long int cutoff = 0;
    long int samplingfreq = 0;
    if ((configline = malloc(STRLENGTH*sizeof(char)))==NULL)
    {
        printf("Cannot allocate config line string\n");
        abort();
    }
    int test;
    FILE *configfile;
    if ((configfile = fopen("config.dat","r"))==NULL)
    {
        printf("Cannot find config file: \"config.dat\"!");
        abort();
    }


    while ((configline = fgets(configline, STRLENGTH, configfile)) != NULL)
    {
        name = strtok(configline,"=");
        value = strtok(NULL,"=\n");
        if (strcmp(name,"readlength") == 0)
        {
            config->readlength = strtoll(value,NULL,10);
        }
        else if (strcmp(name,"start") == 0)
        {
            config->start = strtoll(value,NULL,10);
        }
        else if (strcmp(name,"finish") == 0)
        {
            config->finish = strtoll(value,NULL,10);
        }
        else if (strcmp(name,"cutoff") == 0)
        {
            cutoff = strtol(value,NULL,10);
        }
        else if (strcmp(name,"samplingfreq") == 0)
        {
            samplingfreq = strtol(value,NULL,10);
        }
        else if (strcmp(name,"poles") == 0)
        {
            config->poles = strtol(value,NULL,10);
        }
        else if (strcmp(name,"filterpadding") == 0)
        {
            config->padding = strtol(value,NULL,10);
        }
        else if (strcmp(name,"threshold") == 0)
        {
            config->threshold = strtod(value,NULL);
        }
        else if (strcmp(name,"hysteresis") == 0)
        {
            config->hysteresis = strtod(value,NULL);
        }
        else if (strcmp(name,"numbins") == 0)
        {
            config->numbins = strtol(value,NULL,10);
        }
        else if (strcmp(name,"event_minpoints") == 0)
        {
            config->event_minpoints = strtol(value,NULL,10);
        }
        else if (strcmp(name,"event_maxpoints") == 0)
        {
            config->event_maxpoints = strtol(value,NULL,10);
        }
        else if (strcmp(name,"subevent_minpoints") == 0)
        {
            config->subevent_minpoints = strtol(value,NULL,10);
        }
    }
    config->cutoff = 2.0 *(double) cutoff/(double) samplingfreq;
    free(configline);
    fclose(configfile);
}

int main(int argc, char **argv)
{
    configuration *config;
    if ((config = (configuration *) malloc(sizeof(config)))==NULL)
    {
        printf("Cannot allocate config structure\n");
        abort();
    }

    read_config(config);

    printf("%lld\n%lld\n%lld\n%g\n%ld\n%ld\n",config->start,config->finish,config->readlength,config->cutoff,config->poles,config->padding);
    system("pause");
    return 0;
}

run that in the same directory as the config file I included in the first post, named "config.dat"

comment out the declaration of filepath to see the difference.

[KBriggs]

Also, try using valgrind or electric fence to help you find memory problems.

There is nothing wrong with the code you posted. It is almost certainly elsewhere, in that vast collection of code you didn't post.

Sorry, I missed this. The MWE I posted exhibits the same problem, so the problem has to be somewhere in the sections that I posted.

[anduril462]

malloc(sizeof(config))

config is a pointer, so sizeof(config) is just the size of a pointer, not the size of the object pointed to. Try (note the *)

malloc(sizeof(*config))

Also

• Don't cast malloc.
• Since you malloc configline, then assign fgets to it in your loop, what happens when you're done reading the file and fgets returns NULL? Yep, that's right. You overwrite the malloc'ed pointer in configline with NULL, and have a memory leak. No need to assign, simply check for NULL: while (fgets() != NULL).
• There's no need to malloc configline, unless STRLENGTH is huge (like 1 million). Otherwise, just use a char array, no need to malloc/free.
• You do have a potential divide by 0, as std10093 pointed out.

[KBriggs]

That seemed to be the issue (not allocating correct size for config). Thanks.

And I know I shouldn't cast malloc, but old habits die hard and I catch myself doing it occasionally. Thanks for pointing it out.

I'll fix the rest.

Thanks for your help everyone! Memory problems are strange, sometimes.

[anduril462]

Sorry, I missed this. The MWE I posted exhibits the same problem, so the problem has to be somewhere in the sections that I posted.

No, it doesn't. That's the thing with pointer/memory issues (which often -- as in this case -- result in undefined behavior). They can be caused in one place, and manifest somewhere totally differently. Your function is correct, assuming it gets a valid pointer to a configuration object. If you don't, the problem may manifest in that function, but passing the wrong info into that function is the fault of the calling code.

[KBriggs]

Lesson learned ^_^