passing pointer to func?
Hi,
Anyone can explain me if it is possible to pass pointers to function? I created function which in my opinion should load bmp data to pointer passed to this function
here's code:
/* some data types */
#pragma pack(1)
typedef struct
{
unsigned char r,g,b;
}rgbPIXEL;
pragma pack();
/* and function code : */
void LoadBMPData(char* fname,rgbPIXEL* buffer) //fname - name of bmp file
{
FILE* bmpfile; // file variable
BMPHEADER bmpheader; // header of bmp
unsigned long i = 0; // used in 'for' loop
bmpfile = fopen(fname,"rb"); //
memset(&bmpheader,0,sizeof(bmpheader)); // here I read header data
fread(&bmpheader,1,sizeof(bmpheader),bmpfile); //
/* next I grab memory for buffer : */
buffer = (rgbPIXEL*)malloc(bmpheader.Width*bmpheader.Height*3);
/* here i read data from file, and reverse data order (bgr->rgb) */
for( i = 0; i < bmpheader.Width*bmpheader.Height; ++i )
{
unsigned char r,g,b;
b = fgetc(bmpfile);
g = fgetc(bmpfile);
r = fgetc(bmpfile);
(buffer+i)->r = r;
(buffer+i)->g = g;
(buffer+i)->b = b;
}
fclose(bmpfile); // close file
}
But unfortunally it is not loading data properly :(, i will be grateful for helping me finding whats wrong with it (i'm programming in c)
You need to pass a pointer to the pointer.
/* some data types */#pragma pack(1)typedef struct{unsigned char r,g,b;}rgbPIXEL;pragma pack();/* and function code : */void LoadBMPData(char* fname,rgbPIXEL** buffer) //fname - name of bmp file{FILE* bmpfile; // file variableBMPHEADER bmpheader; // header of bmpunsigned long i = 0; // used in 'for' loopbmpfile = fopen(fname,"rb"); //memset(&bmpheader,0,sizeof(bmpheader)); // here I read header datafread(&bmpheader,1,sizeof(bmpheader),bmpfile); ///* next I grab memory for buffer : */*buffer = (rgbPIXEL*)malloc(bmpheader.Width*bmpheader.Height*3);/* here i read data from file, and reverse data order (bgr->rgb) */for( i = 0; i < bmpheader.Width*bmpheader.Height; ++i ){unsigned char r,g,b;b = fgetc(bmpfile);g = fgetc(bmpfile);r = fgetc(bmpfile);(*buffer+i)->r = r;(*buffer+i)->g = g;(*buffer+i)->b = b;}fclose(bmpfile); // close file}//Usage:rgbPIXEL* data;LoadBMPData("file.bmp", &data);
you need to make buffer a double pointer
a better option would be for the LoadBMPData function to return a rgbPIXEL pointer instead of taking a double pointer as an argument. then clients would do
rgbPIXEL* a = LoadBMPData("file.bmp");
instead of
rgbPIXEL* a;
LoadBMPData("file.bmp", &a);
a better option would be for the LoadBMPData function to return a rgbPIXEL pointer instead of taking a double pointer as an argument. then clients would do
rgbPIXEL* a = LoadBMPData("file.bmp");
instead of
rgbPIXEL* a;
LoadBMPData("file.bmp", &a);
You appear to be working in plain C, so the option of pass-by-reference is not available - thus you would need to "pass by pointer" to make things work as you'd like - this would indeed turn that parameter into a double pointer.
What is going on here is that C passes everything by value, so the "rgbPIXEL* buffer" within the function is not the same rgbPIXEL* variable that was passed in - the former is currently near the top of the stack, while the latter is somewhere else - you got a copy in. Thus when malloc() returns a pointer to newly allocated memory, the caller cannot see that change. (And worse, when the function ends, you leak that memory, because the only pointer to it was on the stack and that's gone now.)
Using a double-pointer solves the problem by telling your function where the original rgbPIXEL* is. Then it can modify that particular bit of memory to point at the new allocation, and all is well. Of course, it is a little bit ugly. In C++ you would do this with a reference (to a pointer), which effectively does the same thing under the hood, but (a) prevents you from having to worry about it, and (b) labels the extra indirection with its specific purpose - passing by reference - instead of just making it an extra level of indirection (which is confusing; it looks like you might be working with a 2D array instead or something).
Of course, as mentioned, you probably want to use the return value instead in this case:
Especially since with the buffer-as-parameter, it's not really clear who's supposed to be responsible for the memory allocation/setup... if I saw a parameter like that, named "buffer", I would expect to have to do the allocation before calling the function, and then the function would gleefully leak the memory I had so carefully provided for it (or tried to... after all, I didn't read the file so I had no idea how much to allocate!)
What is going on here is that C passes everything by value, so the "rgbPIXEL* buffer" within the function is not the same rgbPIXEL* variable that was passed in - the former is currently near the top of the stack, while the latter is somewhere else - you got a copy in. Thus when malloc() returns a pointer to newly allocated memory, the caller cannot see that change. (And worse, when the function ends, you leak that memory, because the only pointer to it was on the stack and that's gone now.)
Using a double-pointer solves the problem by telling your function where the original rgbPIXEL* is. Then it can modify that particular bit of memory to point at the new allocation, and all is well. Of course, it is a little bit ugly. In C++ you would do this with a reference (to a pointer), which effectively does the same thing under the hood, but (a) prevents you from having to worry about it, and (b) labels the extra indirection with its specific purpose - passing by reference - instead of just making it an extra level of indirection (which is confusing; it looks like you might be working with a 2D array instead or something).
/* C */void LoadBMPData(char* fname,rgbPIXEL** buffer) //fname - name of bmp file{FILE* bmpfile; // file variableBMPHEADER bmpheader; // header of bmpunsigned long i = 0; // used in 'for' loopbmpfile = fopen(fname,"rb"); ///* You don't need to memset, you're going to overwrite the data anyway. */fread(&bmpheader,1,sizeof(bmpheader),bmpfile); ///* next I grab memory for buffer : *//* BTW, you shouldn't use the magic number '3' here even if you do know that's the size of your struct... */*buffer = (rgbPIXEL*)malloc(bmpheader.Width*bmpheader.Height*sizeof(rgbPIXEL));/* here i read data from file, and reverse data order (bgr->rgb) */for( i = 0; i < bmpheader.Width*bmpheader.Height; ++i ){unsigned char r,g,b;b = fgetc(bmpfile);g = fgetc(bmpfile);r = fgetc(bmpfile);(*buffer+i)->r = r;(*buffer+i)->g = g;(*buffer+i)->b = b;}fclose(bmpfile); // close file}// C++void LoadBMPData(string filename,rgbPIXEL*& buffer) { ifstream bmpfile(filename.c_str(), ios::in|ios::binary); BMPHEADER bmpheader; bmpfile.read(reinterpret_cast<char*>&bmpheader, sizeof(bmpheader)); buffer = new rgbPIXEL[bmpheader.Width*bmpheader.Height]; for(long i = 0; i < bmpheader.Width*bmpheader.Height; ++i ) { // This works because the get() member function takes a char& - here's the // reference thing working for us again! - and returns *this (allowing // "chaining"). bmpfile.get(buffer.b).get(buffer.g).get(buffer.r); // ICBW but I think you need to skip over the next byte even for 24-bit // (non-alpha) bitmaps... // bmpfile.get(); } bmpfile.close();}
Of course, as mentioned, you probably want to use the return value instead in this case:
// C++. I'm sure you can figure it out for C.rgbPIXEL* LoadBMPData(string filename) { ifstream bmpfile(filename.c_str(), ios::in|ios::binary); BMPHEADER bmpheader; bmpfile.read(reinterpret_cast<char*>&bmpheader, sizeof(bmpheader)); // Now we need to make the variable ourselves, and return it at the end. rgbPIXEL* buffer = new rgbPIXEL[bmpheader.Width*bmpheader.Height]; for(long i = 0; i < bmpheader.Width*bmpheader.Height; ++i ) { // This works because the get() member function takes a char& - here's the // reference thing working for us again! - and returns *this (allowing // "chaining"). bmpfile.get(buffer.b).get(buffer.g).get(buffer.r); // ICBW but I think you need to skip over the next byte even for 24-bit // (non-alpha) bitmaps... // bmpfile.get(); } bmpfile.close(); return buffer; // There we go. Nice and clean.}
Especially since with the buffer-as-parameter, it's not really clear who's supposed to be responsible for the memory allocation/setup... if I saw a parameter like that, named "buffer", I would expect to have to do the allocation before calling the function, and then the function would gleefully leak the memory I had so carefully provided for it (or tried to... after all, I didn't read the file so I had no idea how much to allocate!)
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement