malloc for struct

[vaneric001]

hi
I am a newbie to c coding and going thru K&R ...
i have a structure to represent an image as

struct image{
char* imagename;
int imagewidth;
int imageheight;
long* pixels;
};

typedef struct image img;
typedef img* imgptr;

here long * pixels shd point to an array of long values representing
pixels of the image.Arraysize (ie number of pixels )will be known only
at runtime.i want to read values into this struct and return a pointer
so i can use this later in another fn

so i wrote
imgptr readimage(char* filename){
/* read image with some library and set wifth,height and get
pixel data array (obtained like long* pixeldata)*/
....
/* allocate memory for structure */
imgptr thisimageptr=malloc(sizeof(thisimageptr));
if(thisimageptr==NULL) exit(EXIT_FAILURE);
int numberofpixels=width * height;
/*allocate memory for the long * pixels */
thisimageptr->pixels=malloc(sizeof(long*) * numberofpixels);
if(thisimageptr->pixels==NULL) exit(EXIT_FAILURE);
/* set fields of struct */
thisimageptr->imagename=filename;
thisimageptr->imagewidth=width;
thisimageptr->imageheight=height;

for(i=0;i<numberofpixels;i++){
thisimageptr->pixels[i]=pixeldata[i];;
}

return thisimageptr;
}
if another fn gets this pointer it should be able to retrieve the
fields and should be able to iterate over the pixelvalues

i am a little doubtful about the malloc part..is the above mallocs
correct ? or is there some hidden risk?

[pete]

va********@gmail.com wrote:

>
hi
I am a newbie to c coding and going thru K&R ...
i have a structure to represent an image as

struct image{
char* imagename;
int imagewidth;
int imageheight;
long* pixels;
};

typedef struct image img;
typedef img* imgptr;

imgptr thisimageptr=malloc(sizeof(thisimageptr));

i am a little doubtful about the malloc part..is the above mallocs
correct ?

Unlikely.

or is there some hidden risk?

This is more likely to be correct:

imgptr thisimageptr = malloc(sizeof *thisimageptr);

This is more likely to be correct and better style:

img *thisimageptr = malloc(sizeof *thisimageptr);

--
pete

[vaneric001]

>
This is more likely to be correct:

imgptr thisimageptr = malloc(sizeof *thisimageptr);

This is more likely to be correct and better style:

img *thisimageptr = malloc(sizeof *thisimageptr);

pete
thanx for the reply
what i was worried about was
when i allocate for that pixels
do i have to allocate memory for a size of (a long pointer)*(number of
items in the array) ?
only then can it be big enough to hold all those long pointers

eric

[pete]

va********@gmail.com wrote:

thisimageptr->pixels=malloc(sizeof(long*) * numberofpixels);

i am a little doubtful about the malloc part..is the above mallocs
correct ? or is there some hidden risk?

That way is OK, but this way is better style:

thisimageptr->pixels
= malloc(sizeof *thisimageptr->pixels * numberofpixels);

http://c-faq.com/malloc/noscale.html

--
pete

[pete]

va********@gmail.com wrote:

>

This is more likely to be correct:

imgptr thisimageptr = malloc(sizeof *thisimageptr);

This is more likely to be correct and better style:

img *thisimageptr = malloc(sizeof *thisimageptr);

pete
thanx for the reply
what i was worried about was
when i allocate for that pixels
do i have to allocate memory for a size of (a long pointer)*(number of
items in the array) ?
only then can it be big enough to hold all those long pointers

That's right.
An array of pointers to arrays is commonly used that way.

--
pete

[jimgardener]

do i have to allocate memory for a size of (a long pointer)*(number of
items in the array) ?
only then can it be big enough to hold all those long pointers

That's right.
An array of pointers to arrays is commonly used that way.

pardon me if i am not following this correctly..
the long pointer
thisimageptr->pixels points to a chunk that shd be big enough to
hold (numberofpixels) values which are 'long's not 'long*'s
so the allocation should be to a size equal to numberofpixels*
sizeof(long)

i believe that is what happens in
thisimageptr->pixels
= malloc(sizeof *thisimageptr->pixels * numberofpixels);

on the other hand ,in
thisimageptr->pixels
= malloc(sizeof(long*) * numberofpixels);
allocation happens to multiple of sizeof(long*).
that will give correct result only if
sizeof(long*) ==sizeof(long) in a machine

am i correct..or is my understanding wrong?
jim

[pete]

jimgardener wrote:

>

do i have to allocate memory for a size of
(a long pointer)*(number of
items in the array) ?
only then can it be big enough to hold all those long pointers

That's right.
An array of pointers to arrays is commonly used that way.

pardon me if i am not following this correctly..
the long pointer
thisimageptr->pixels points to a chunk that shd be big enough to
hold (numberofpixels) values which are 'long's not 'long*'s
so the allocation should be to a size equal to numberofpixels*
sizeof(long)

i believe that is what happens in
thisimageptr->pixels
= malloc(sizeof *thisimageptr->pixels * numberofpixels);

on the other hand ,in
thisimageptr->pixels
= malloc(sizeof(long*) * numberofpixels);
allocation happens to multiple of sizeof(long*).
that will give correct result only if
sizeof(long*) ==sizeof(long) in a machine

am i correct..or is my understanding wrong?
jim

You are correct in what you have said about allocation
for each pointer in the array of pointers.
But I believe that the topic being addressed by my reply to OP
was about the allocation of the array of pointers.
After which each pointer in the array would
then be pointed to allocated memory.

--
pete

[Malcolm McLean]

<va********@gmail.comwrote in message news

>

>>
This is more likely to be correct:

imgptr thisimageptr = malloc(sizeof *thisimageptr);

This is more likely to be correct and better style:

img *thisimageptr = malloc(sizeof *thisimageptr);

pete
thanx for the reply
what i was worried about was
when i allocate for that pixels
do i have to allocate memory for a size of (a long pointer)*(number of
items in the array) ?
only then can it be big enough to hold all those long pointers

struct image *answer;
answer = malloc(sizeof(struct image));
if(!answer)
return 0;
/* get width and height somehow */
answer->pixels = malloc(sizeof(long) * width * height);
if(!answer->pixels)
{
free(answer);
return 0;
}
/* fill answer->pixels with raster data, somehow */
return answer;

Some advanced programmers will say malloc(sizeof *answer->pixels * width *
height) instead, so that if you change the pixel representation the code
will update. However in my opinion this is hard to read, and has proved a
source of confusion to you.
sizeof() may take either a type name or a variable.

In this case, the allocation is likely to be of a meduim to large size,
depending on the image and your system. So it is forseeable that it might
fail, and you should be prepared to handler failure conditions in the
caller.
It is most unlikely that the trivial allocation for the structure itself
will fail, but since we are already reporting out of memory conditions, we
might as well report it as well.

--
Free games and programming goodies.
http://www.personal.leeds.ac.uk/~bgy1mm