malloc() and free() an array in c

Question

I have been using Java and quite new to C. I tried to create a function that generates a random pixel array with malloc. I free the memory in another function after using that random array. I think my concept is just fine, but I wonder if I write the codes properly and it really frees the heap memory. It'd be great if you can look at the codes and see if it works.

pixel* randomPalette(int colors){

    int i, x;
    pixel * randomArr = (pixel *)malloc(sizeof(pixel)* colors);
    srand(time(NULL)); //generate a random seed
    for (i = 0; i < colors; i++){
        x = rand() % 256;
        randomArr[i].r = x; randomArr[i].g = x; randomArr[i].b = x;
    }
    return randomArr;
}

void QuantizeA(pixel* Im, int width, int height){
    //create a random palette of 8 RGB colors;
    const int num = 8;
    pixel* Arr = randomPalette(num);

    //find the min distance between pixel and palette color
    int x, y, z;
    int min = 195075; // max distance is 255^2 + 255^2 + 255^2
    int pos = 0;
    for (x = 0; x < height; x++){
        for (y = 0; y < width; y++){
            //compare distance of the pixel to each palette color
            for (z = 0; z < num; z++) {
                if (distance(Im[pos], Arr[z]) < min){
                    Im[pos].r = Arr[pos].r;
                    Im[pos].g = Arr[pos].g;
                    Im[pos].b = Arr[pos].b;
                }
            }
            pos++; //go to next piexl
        }
    }
    glutPostRedisplay();
    free(Arr);
}

[Please see this discussion on why not to cast the return value of `malloc()` and family in `C`.](http://stackoverflow.com/q/605845/2173917). — Sourav Ghosh, Sep 27 '15 at 19:35
And check if `malloc()` returns `NULL`, there are no exceptions in c so you might like to be careful. Also, what is the question? Stack Overflow is not meant to check the correctness of working code, if it fails somehow please specify how it fails, otherwise search for the appropriate site for your question, there is a [Code Review](http://codereview.stackexchange.com/) site too. — Iharob Al Asimi, Sep 27 '15 at 19:36
I have to cast the return value of malloc() in this case otherwise it somehow comes error. — MLAC, Sep 27 '15 at 19:40
It sounds like you're compiling this as C++ (.cpp suffix) rather than C (.c suffix) ? — Paul R, Sep 27 '15 at 20:56
@MLAC figure out how to invoke your compiler in C mode. Then move `srand(time(NULL))` out of where it is. Read up on random seeding, you only want to do it once per program typically. The way you have it now, you will keep getting the same "random" value if you call this function multiple times within the same second. — M.M, Sep 27 '15 at 22:47

score 1 · Answer 1 · answered Sep 27 '15 at 19:39

1

From the memory allocation-deallocation part, your code is fine (I did not check your logic). However, two things to notice here,

Check for the success of malloc() before using the returned value.
You need to call srand(time(NULL)); only once at the start of your program, possibly in main().

As a suggestion, you can always make use of the memcheck tool from valgrind to check for the memory leakage related issues, if you suspect any.

Also, please see this discussion on why not to cast the return value of malloc() and family in C..

answered Sep 27 '15 at 19:39

Sourav Ghosh

133,132
16
183
261

Thanks. I tried not to cast the return value of malloc() but somehow it got error. If I cast it to (pixel *) it works fine. – MLAC Sep 27 '15 at 19:44
2

@MLAC are you sure you're using a `C` compiler? – Sourav Ghosh Sep 27 '15 at 19:45
I know.... I'm using the template from the prof. I guess he must have screwed up the file format when he updated it from VisualStudio2012 version to 2013. In his template, he also casts the malloc() return value too. – MLAC Sep 27 '15 at 20:47

score 1 · Answer 2 · answered Sep 27 '15 at 19:42

When you assign the new minimum distance:

Im[pos].r = Arr[pos].r;

you use the wrong index. It should be:

Im[pos].r = Arr[z].r;

As a side note: You cannot compare two structs with the comparison operators, not even for equality, but C allows you to assign one struct to another, which effectively copies the contents. So you don't need to copy all components, you can just say:

Im[pos] = Arr[z];

malloc() and free() an array in c

2 Answers2