Phil O'kelly
Phil O'kelly

Reputation: 171

Using pointers in 2D arrays

I'm attempting to store arrays of integers that I read from a file (with a separate function) in a 2D array but I keep having issues with Segmentation fault. I know it's an issue with my pointers but I can't figure out exactly what I'm doing wrong.

Here is my function (takes an integer and compares it with an integer read from a file before storing it in my 2D array).

int **getStopTimes(int stop_id) {

int **result = malloc(sizeof(*result)); 
char const* const fileName = "stop_times_test.txt"; 
FILE* txt = fopen(fileName, "r"); 
char line[256];
int count = 0;

while (fgets(line, sizeof(line), txt) != NULL) {    
        int *formattedLine = getStopTimeData(line); //getStopTimeData returns a pointer to an array of ints, memory is allocated in the function
        if (formattedLine[1] == stop_id) {
            result[count] = formattedLine;
            count++;
        }                           
}       
fclose(txt);
return result;  
}

And my main:

int main(int argc, char *argv[]) {
int **niceRow = getStopTimes(21249);
for (int i=0; i<2; i++) { //Only looping 3 iterations for test purposes
    printf("%d,%d,%d,%d\n",niceRow[i][0], niceRow[i][1], niceRow[i][2], niceRow[i][3]);
}
free(niceRow);
return 0;
}

getStopTimeData function thats being called (Pulls certain information from an array of chars and stores/returns them in an int array):

int *getStopTimeData(char line[]) {
int commas = 0;
int len = strlen(line);
int *stopTime = malloc(4 * sizeof(*stopTime)); //Block of memory for each integer
char trip_id[256]; //Temp array to build trip_id string
char stop_id[256]; //Temp array to build stop_id string
int arrival_time; //Temp array to build arrival_time string 
int departure_time; //Temp array to build departure_time string 
int counter;

for(int i = 0; i <len; i++) { 
    if(line[i] == ',')  {
        commas++;
        counter = 0;
        continue;
    }
    switch(commas) { //Build strings here and store them 
        case 0 : 
            trip_id[counter++] = line[i]; 
            if(line[i+1] == ',') trip_id[counter] = '\0';
            break;
        case 1: //Convert to hours past midnight from 24hr time notation so it can be stored as int
            if(line[i] == ':' && line[i+3] == ':') {
            arrival_time = (line[i-2]-'0')*600 + (line[i-1]-'0')*60 + (line[i+1]-'0')*10 + (line[i+2]-'0'); 
            }   
            break;
        case 2 : 
            if(line[i] == ':' && line[i+3] == ':') {
            departure_time = (line[i-2]-'0')*600 + (line[i-1]-'0')*60 + (line[i+1]-'0')*10 + (line[i+2]-'0');
            }       
            break;
        case 3 : 
            stop_id[counter++] =  line[i];
            if(line[i+1] == ',') stop_id[counter] = '\0';
            break;
    }
}
//Assign and convert to ints
stopTime[0] = atoi(trip_id);
stopTime[1] = atoi(stop_id);
stopTime[2] = arrival_time;
stopTime[3] = departure_time;
return stopTime;
}

Upvotes: 2

Views: 117

Answers (2)

Thomas Kostas
Thomas Kostas

Reputation: 455

The upper answer is good, just to give you an advice try to avoid using 2D array but use a simple array where you can store all your data, this ensures you to have coalescent memory.

After that, you can access your 1D array with an easy trick to see it like a 2D array

Consider that your 2D array has a line_size

To access it like a matrix or a 2d array you need to find out the corresponding index of your 1d array for given x,y values

index = x + y * line size;

In the opposite way: you know the index, you want to find x and y corresponding to this index.

y = index / line_size;
x = index mod(line_size);

Of course, this "trick" can be used if you already know your line size

Upvotes: 0

user2371524
user2371524

Reputation:

This line:

int **result = malloc(sizeof(*result));

allocates just memory for one single pointer. (*result is of type int *, so it's a pointer to data -- the sizeof operator will tell you the size of a pointer to data ... e.g. 4 on a 32bit architecture)

What you want to do is not entirely clear to me without seeing the code for getStopTimeData() ... but you definitely need more memory. If this function indeed returns a pointer to some ints, and it handles allocation correctly, you probably want something along the lines of this:

int result_elements = 32;
int **result = malloc(sizeof(int *) * result_elements);
int count = 0;

[...]
    if (formattedLine[1] == stop_id) {
        if (count == result_elements)
        {
            result_elements *= 2;
            result = realloc(result, result_elements);
        }
        result[count] = formattedLine;
        count++;
    }

Add proper error checking, malloc and realloc could return (void *)0 (aka null) on out of memory condition.

Also, the 32 for the initial allocation size is just a wild guess ... adapt it to your needs (so it doesn't waste a lot of memory, but will be enough for most use cases)

Upvotes: 2

Related Questions