Using pointers in 2D arrays

Question

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;
}

Also, post the code for getStopTimeData(). If it's calling malloc() or similar, if it's wrong it can corrupt the heap, with SEGVs appearing elsewhere in your code. — Andrew Henle
– Andrew Henle, Commented Sep 18, 2015 at 0:24
regarding this line: int **result = malloc(sizeof(*result)); 1) it mallocs either 4 or 8 bytes depending on the underlying architecture. 2) always check (!=NULL) the returned value from malloc() to assure the operation was successful — user3629249
– user3629249, Commented Sep 18, 2015 at 1:37
regarding this line: int main(int argc, char *argv[]) Will cause the compiler to output 2 warnings, 1) unused parameter 'argc' 2) unused parameter 'argv[]' Suggest fixing that by declaring main as: int main( void ) — user3629249
– user3629249, Commented Sep 18, 2015 at 1:40
please, for readability by us humans and for easy understandability, indent code consistently (and never use tabs for indenting as each wordprocessor/editor has the tab stops/tab width set differently) Suggest 4 spaces after every opening brace '{' (4 spaces is wide enough to be visible even with variable width fonts) and un-indent before every closing brace '}' — user3629249
– user3629249, Commented Sep 18, 2015 at 1:42

user2371524 · Accepted Answer · 2015-09-18 00:26:20Z

2

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)

answered Sep 18, 2015 at 0:26

user2371524

Sign up to request clarification or add additional context in comments.

2 Comments

Phil O'kelly Over a year ago

Modified my code accordingly and changed around the allocation size but unfortunately I just keep getting the same segmentation error. I'm not sure what it means but as soon as I add printf anywhere in my function it runs perfectly fine and gives the output I need - though obviously this isnt fixing the issue

user2371524 Over a year ago

@PhilO'kelly ... you have two options now. 1.) reduce your code to a minimal but compilable example that reproduces the error -- chances are you will find the remaining error yourself in the process. If not, post it in your question. 2.) use some tools specially tailored to find memory management problems -- if you're on a *nix system, try valgrind (compile with -O0 -g3 if using gcc or a compatible compiler). Look carefully at the output. If you still don't identify the root problem, add the output to your question.

Thomas Kostas · Accepted Answer · 2018-03-07 02:02:40Z

0

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

edited Mar 7, 2018 at 2:02

answered Sep 18, 2015 at 0:46

Thomas Kostas

4654 silver badges11 bronze badges

4 Comments

user2371524 Over a year ago

Note this suggestion is very dependent on the structure of your data. It only makes sense if you know a maximum row length (you call it line size here) and at the same time ensure that the actual row lengths don't deviate to much. In THAT case, it's a good suggestion -- otherwise it's just wasting memory.

user2371524 Over a year ago

And adding to that, I disagree with "try to avoid using 2D array". That's because the 2D array is probably the generic solution to the OPs problem. There's a simple rule of thumb software engineers should know about: hardware is cheap, software is expensive. So, if "flattening" a 2D array into a "simple" array really makes a big difference, it is worth the effort. Otherwise, it isn't.

Thomas Kostas Over a year ago

Actually it's not an effort it's way more simple to manipulate than double pointers...

user2371524 Over a year ago

No, it's not. The pointer semantics in C might appear quite strange at first, but please take a broader look at it. Choosing a data structure (in any language) should at first depend on how the real-world data you want to model is structured. The C approach to operate on pointers makes a 2D array look complicated at first, but once you get used to the syntax involved, it's no more complicated than e.g. a List<List<T>> in C#. Modelling data structures technically different from their natural form (as far as the language allows) is premature optimization.

Collectives™ on Stack Overflow

Using pointers in 2D arrays

2 Answers 2

2 Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

2 Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related