-1

apologies if this has appeared elsewhere, I've not been able to find a clear answer. I've been using Ed S's answer, Option 1 (linked below) to allocate memory, populate the array, then return it back to the caller. He recommends freeing the memory after you've finished with it, however when I added the free() line, I get a core dump. I've had a poke around with GDB, but my skills probably aren't what the need to be.

Thanks in advance for any help you can give.

Link to answer: Returning an array using C

Code:

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

char * createArray();

int main(int argc, char *argv[]){
    printf("Creating Array...\n");
    // pointer to an int
    char *p;
    // size of the array
    int i,j;
    // width of array
    int width = 7;
    // height of array
    int height = 5;
    // get an array from the function
    p = createArray(width, height);
    // check if p was created properly
    if (p){
        // print out the array
        for (i = 0; i < width; ++i){
            for (j = 0; j < height; ++j){
                printf("[%c] ", *(p + (i * width) + j)); 
            }
        printf("\n");
        }

        // here's where it hits the fan
        free(p);
    }
    return 0;
}

char * createArray(int w, int h){
    // allocate some memory for the array
    char *r = malloc(w * h * sizeof(char));
    // check if the memory allocation was successful
    if(!r){
        return NULL;
    }
    int i, j;
    int count = 0;
    for (i = 0; i < w; ++i){
        for (j = 0; j < h; ++j){
            *(r + (i * w) + j) = 'X';
            ++count;
        }
    }
    return r;
}
dleft
  • 73
  • 1
  • 4
  • 2
    Are you on a platform where [Valgrind](http://valgrind.org/) works? If so, use it. The core dump likely means you've trampled outside the bounds of your allocated array. – Jonathan Leffler Sep 20 '17 at 17:37
  • 3
    Why do people (particularly novices, I notice), like `*(r + (i * w) + j)` instead of `r[i * w + j]`? Why is that nested loop counting? You never use the count. Are you meant to be creating strings? You have no null terminators anywhere, so that you're _not_ creating strings, just character arrays, – Jonathan Leffler Sep 20 '17 at 17:39
  • @JonathanLeffler - I'll check out valgrind now, I've not done much debugging but it's an essential tool to learn so now is as good a time as any! - Not sure really, I just used that because that's how I learned to do it. - Lastly, the count was there because I originally had the array contain ints, i wanted to check I had created the array properly so each element had a seperate value. When I changed it to chars I just forgot to remove the count. – dleft Sep 20 '17 at 17:44
  • 1
    This `*(r + (i * w) + j) = 'X'` should have been `*(r + (i * h) + j) = 'X'` – hesham_EE Sep 20 '17 at 17:45
  • And there is a similar error in `main`: the expression `*(p + (i * width) + j)` should be `*(p + (i * height) + j)`. – John Bollinger Sep 20 '17 at 17:47
  • @hesham_EE Just tried your response, and it's now not populating the array correctly. Output: [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [X] [] [] [] [] [] [] [] [] [] [] – dleft Sep 20 '17 at 17:49
  • "but my skills probably aren't what the need to be." - That should tell you to start with something more basic to get the skills. A good textbook might help, too. – too honest for this site Sep 20 '17 at 17:53
  • @Olaf for sure. I've been doing 1D arrays and had no trouble, so I thought I would take a crack at 2D arrays. Probably running before I can walk! – dleft Sep 20 '17 at 17:56
  • @dleft this is because you check the populated elements with the wrong indices. You may need to fix this loop the same way I commented before about the other loop. – hesham_EE Sep 20 '17 at 17:57
  • @dleft: I don't see any 2D array in your code. After all, it should be clear the cannot have the same declarator like 1D arrays or pointers. One on the road: A pointer is not an array and an array is not a pointer. That's vital to remember in C. Whoever tells different should not write production code in C. – too honest for this site Sep 20 '17 at 17:59

2 Answers2

2

With this

char *r = malloc(w * h * sizeof(char));

You allocate w * h (7 * 5 = 35 bytes) of memory. But

        *(r + (i * w) + j) = 'X';

can access well beyond the 35 bytes you have allocated (you'll see if you test the possible values for i * w + j in the loop), resulting in undefined behaviour.

This possibly overwrites the malloc's internal data structures and thus you happen to get core dump when you free().

P.P
  • 117,907
  • 20
  • 175
  • 238
  • 1
    Yup, and Valgrind confirms this diagnosis. And there's the corresponding problem in `main()` too in the printing loop. – Jonathan Leffler Sep 20 '17 at 17:48
  • thanks for the response! I'll take @JonathanLeffler's advice now and try to learn valgrind. Thanks! – dleft Sep 20 '17 at 17:51
  • @dleft: the main thing to remember with Valgrind is to compile _and_ link with the `-g` option, so you get told the line numbers where the problems are occurring. – Jonathan Leffler Sep 20 '17 at 17:53
  • @JonathanLeffler thanks for the advice! Looking through the docs for Valgrind now. When I've worked how I can fix the code I will post the solution at the bottom of my question. – dleft Sep 20 '17 at 17:57
1

You made a mistake on these lines

*(r + (i * w) + j) = 'X';

and

printf("[%c] ", *(p + (i * width) + j));

To keep inside the boundaries of your "2D" array -it's one dimensional but you are working around it like a compiler would-it should be i * length in there:

*(r + (i * h) + j) = 'X';`

and 

printf("[%c] ", *(p + (i * height) + j)); `

If you use this, you should be able to stay within the boundaries without making a mess.

hassan arafat
  • 657
  • 5
  • 15
  • Thanks for the tip! I've got it up and running now, but I really should do a little more work on traversing 2D arrays before continuing with this! – dleft Sep 21 '17 at 09:30