Char buffer and 3x int in struct

Question

I am doing some tests in C (& Java) and one benchmark. I've got code for image deform (from my first answer) and I am rewriting it to C. I want to compare the speed. I"ve done simple image "codec" - colors are stored without compression, RGB (or BGR?) and struct called OneArrayImage (ArrayImage = OneArrayImage*) with some methods and there is an error. When i try to create instance of struct with char* and 3x ints, these data are broken (eg. w = 0, len = -24562...). How should I adjust this struct and methods (ArrayImage image(4) and ArrayImage image_empty(2)) to work? Thanks.

.

(2 main Sources:)

Image.c: http://pastebin.com/c0ZmuRbU

Deformer.c: http://pastebin.com/kCbkGSzm

.

An Error might be in:

struct OneArrayImage {
    int* w;
    int* h;
    int* len;
    unsigned char* pix;
};
typedef struct OneArrayImage* ArrayImage;


/*** Prepare ***/



/*** New ***/

ArrayImage image(char* pix, int len, int w, int h) {
    if(w < 1 || h < 1 || len < 1 || pix == NULL) {
        return NULL;
    }
    int len2 = (w * h * 3);
    if(len2 > len) {
        return NULL;
    }

    struct OneArrayImage img = {&w, &h, &len2, pix};
    ArrayImage ret = &img;
    return ret;
}

ArrayImage image_empty(int w, int h) {
    if(w < 1 || h < 1) {
        return NULL;
    }
    int len = (w * h * 3);
    unsigned char* pix = (unsigned char*) malloc(len);

    struct OneArrayImage img;// = {&w, &h, &len, pix};
    img.w = &w;
    img.h = &h;
    img.len = &len;
    img.pix = pix;
    ArrayImage ret = &img;
    return ret;
}

main() is in Deformer.c, output:

Starting!
S I  : 830835 0
PIX  : NOT_NULL
IMG L: NOT_NULL
IMG E: NOT_NULL
PIX L: NULL
PIX E: NULL

.

Working EDIT

/*** Typedef ***/

struct OneArrayImage {
    int w;
    int h;
    int len;
    unsigned char* pix;
};
typedef struct OneArrayImage* ArrayImage;


/*** Prepare ***/



/*** New ***/

ArrayImage image(char* pix, int w, int h) {
    if(w < 1 || h < 1 || pix == NULL) {
        return NULL;
    }
    int len = (w * h * 3);

    ArrayImage image = (ArrayImage) malloc(sizeof(struct OneArrayImage));
    image->w = w;
    image->h = h;
    image->len = len;
    image->pix = pix;
    return image;
}

ArrayImage image_empty(int w, int h) {
    if(w < 1 || h < 1) {
        return NULL;
    }
    int len = (w * h * 3);
    unsigned char* pix = (unsigned char*) malloc(len);

    ArrayImage image = (ArrayImage) malloc(sizeof(struct OneArrayImage));
    image->w = w;
    image->h = h;
    image->len = len;
    image->pix = pix;
    return image;
}

unwind · Answer 1 · 2014-08-14T15:00:33.403

You are returning the address of a local variable. When the function ends, that variable goes out of scope, and accessing its old address is an illegal operation that will trigger undefined behavior.

You must use malloc() to allocate heap memory that will persist.

Also, I recommend against typedef:ing away the pointer asterisk in C, it's very confusing. The scalar fields (size) should not be int *, they're not pointers!

You should do something like:

struct OneArrayImage {
  int w;
  int h;
  int len;
  unsigned char* pix;
};

struct OneArrayImage * image(char* pix, int len, int w, int h)
{
  struct OneArrayImage *img = malloc(sizeof *img);
  if(img != NULL)
  {
   img->w = w;
   img->h = h;
   img->len = 3 * w * h * len;
   img->pix = pix;
  }
  return img;
}

Thank you, but gamernb was first. This answer is similar to his. +1 — Yep Possible, Aug 14 '14 at 14:59

Deduplicator · Answer 2 · 2014-08-14T14:54:16.663

struct OneArrayImage {
    int* w;
    int* h;
    int* len;
    unsigned char* pix;
};

Look carefully at the structure. You don't actually want pointers for the first three elements, but int values.

When you later in your code ran against that, you improperly assigned the address of automatic variables instead of correcting your types:

ArrayImage image(char* pix, int len, int w, int h) {
// [...[
    struct OneArrayImage img = {&w, &h, &len2, pix};

Those automatic variable go out of scope at the end of the function, so you get dangling pointers meaning undefined behavior.

BTW: You have the exact same problem of dangling pointers with your return type. Allocate some memory for it please, will you?

struct OneArrayImage img;
ArrayImage ret = &img;
return ret; // returning a dangling pointer

You have the choice between:

static memory (re-entrancy and concurrency issues)
dynamic memory (malloc and free)
Caller-supplied memory (like e.g. returning the struct itself instead of a pointer to it)

score 1 · Accepted Answer · answered Aug 14 '14 at 14:50

You have a basic problem in how you design your struct:

struct OneArrayImage {
    int* w;
    int* h;
    int* len;
    unsigned char* pix;
};

The width, height, and length variables should just be int not a pointer to an int (int *). Later, when you are trying to calculate len2 here:

int len2 = (w * h * 3);

You are actually multiplying pointers. Also when you are setting them, you are assigning to them pointer of temporary memory:

ArrayImage image(char* pix, int len, int w, int h) {
    if(w < 1 || h < 1 || len < 1 || pix == NULL) {
        return NULL;
    }
    int len2 = (w * h * 3);
    if(len2 > len) {
        return NULL;
    }

    struct OneArrayImage img = {&w, &h, &len2, pix};
    ArrayImage ret = &img;
    return ret;
}

Finally, above, when you return, you are returning a pointer to 'img', which is bad because you also are using temporary memory there as well. In order to return the pointer, you must allocate it (and later free it). For example:

ArrayImage image = (ArrayImage)malloc(sizeof(struct OneArrayImage));
image->w = w;
image->h = h;
image->len = len2;
image->pix = pix;
return image;

[Don't cast the return value of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Drew McGowen, Aug 14 '14 at 14:51
Also, the `len2` calculation doesn't involve multiplying pointers; `w` and `h` come from parameters, which are just `int`. — Drew McGowen, Aug 14 '14 at 14:53

Char buffer and 3x int in struct

3 Answers3