Odd C pointer segmentation fault errors

Question

I am writing a program to split up data entered via command line and put it into separate fields. Right now I am having problems splitting up the data and assigning it to respective pointers. The text file is this:

5, 08:00:00, 2, 60

Where 5 is the patient number, the reading was at 8am, and it is field 2 and he got a 60. I keep getting a segmentation fault error when I run my code, so I put it though gdc and got this, and I think line 88:

*hours = atoi(j);

is messing up:

Welcome to the Health Monitoring System

Program received signal SIGBUS, Bus error. 0x0000000000400826 in get_field ()

This is my code

/*
* Health Monitoring System
*/

#include <stdio.h>
#include <ctype.h>

#define MAXPATIENTS 5
#define MAXREADINGS 10
#define MAXTYPES 5
#define MAXTIME 8

/* One health type reading: timestamp + actual value */
typedef struct{
        char timestamp[MAXTIME+1];
        int value;
}Element;

/* Circular buffer of health type readings */
typedef struct{
        int start;      /* index of oldest reading */
        int end;        /* index of most current reading */
        Element reading[MAXREADINGS];
}CircularBuffer;

/* Patient's health chart: ID + multiple health type readings */
typedef struct{
        int id;
        CircularBuffer buffer[MAXTYPES];
}Chart;


/*
* Health records for all patients defined here.
* The variable record is visible to all functions
* in this file, i.e. it is global.
*/
Chart record[MAXPATIENTS];

void main(){
int i, j;

/* initialize health data records for each patient */

for( i=0; i < MAXPATIENTS; i++ ){
    record[i].id = i + 1;
    for( j=0; j < MAXTYPES; j++ ){
        record[i].buffer[j].start = 0;
        record[i].buffer[j].end = 0;
        }
    }
printf("Welcome to the Health Monitoring System\n\n");

int id;
int hours;
int mins;
int secs;
int field;
int score;
get_field(&id, &hours, &mins, &secs, &field, &score);
printf("%d This is the ID\n", id);
printf("%d This is the hours\n", hours);
printf("%d This is the mins\n", mins);
printf("%d This is the secs\n", secs);
printf("%d This is the field\n", field);
printf("%d This is the score\n", score);

printf("\nEnd of Input\n");
}



int get_field(int* id, int* hours, int* mins, int* secs, int* field, int* score){
        //get the patient ID
        int z = getchar();
        *id = z;
        getchar();
        getchar();

        //this gets the hour

        char j[MAXTIME];
        int m,n = 0;
        while((n=getchar()) != ':'){
                j[m] = n;
                m++;
        }
        *hours = atoi(j);

        //this gets the mins

        char k[MAXTIME];
        n = 0;
        m = 0;
        while((n=getchar()) != ':'){
                k[m] = n;
                m++;
        }
        *mins = atoi(k);

        // this gets the seconds

        char l[MAXTIME];
        n = 0;
        m = 0;
        while((n=getchar()) != ':'){
                l[m] = n;
                m++;
        }
        *secs = atoi(l);

        getchar();
        getchar();

        // this gets the field

        z = getchar();
        *field = z;
        getchar();
        getchar();

        // this gets the score

        m = 0;
        n = 0;
        char x[MAXTIME];
        while ((n=getchar()) != '\n'){
                x[m] = n;
                m++;
        }
        *score = atoi(x);
        return 0;
}

Answer 1

You're not zero-terminating the strings you're building; atoi() may be reading past the ends of the arrays.

// x will be uninitialized, not necessarily zero-filled
char x[MAXTIME];
while ((n=getchar()) != '\n'){
  x[m] = n;
  m++;
}

x[m] = '\0';    // make it a valid C string

*score = atoi(x);

All of this assumes that we don't get more than MAXTIME characters.

To avoid that problem:

while ((m < (MAXTIME - 1)) && ((n=getchar()) != '\n')){

Answer 2

I would use scanf instead of running it manually...

 scanf("%d, %d:%d:%d, %d, %d", &field1, &hour, &min, &sec, &field2, &field3);

That would probably clean up some of the problems you are having. Hope that helps.

The problem is, somewhere in that mess of get_field, you are running into an error you don't need to have. scanf uses what are called format strings, meaning they match to a SPECIFIC format and insert data into their respective fields. This takes the pain away from the parsing you are needlessly doing and makes it a lot easier when you can debug the hard stuff instead of trivial stuff like that.

Answer 3

You'll have problems with the following lines:

*hours = atoi(j);

*mins = atoi(k);

*secs = atoi(l);

*score = atoi(x);

since you are not terminating the strings with a null character before calling atoi.