debugcMinor
BMP file writer
Viewed 0 times
writerfilebmp
Problem
In my current project I'm writing a C function to write a BMP-File. Internally, the function is split up into three parts: write header, write info header, write data.
In some of these functions I do need to allocate heap space with
Now I'm unsure of how to handle failure situations reasonably. My current approach is to have a returned integer value indicate success.
```
uint8_t BMP32_write(FILE output, BMP32_FILE bmp32_file) {
uint8_t res = 0;
res |= BMP32_write_header(output, bmp32_file->bmph);
res |= BMP32_write_info_header(output, bmp32_file->bmpih);
res |= BMP32_write_rgbData(output, bmp32_file->bmpih->biWidth, bmp32_file->bmpih->biHeight, bmp32_file->rgbData);
return res;
}
static uint8_t BMP32_write_header(FILE output, BMP32_HEADER h) {
uint8_t res = 0;
res |= !fwrite(&h->bfType , sizeof(h->bfType) , 1, output);
res |= !fwrite(&h->bfSize , sizeof(h->bfSize) , 1, output);
res |= !fwrite(&h->bfReserved, sizeof(h->bfReserved), 1, output);
res |= !fwrite(&h->bfOffBits , sizeof(h->bfOffBits) , 1, output);
return res;
}
static uint8_t BMP32_write_info_header(FILE output, BMP32_INFO_HEADER ih) {
uint8_t res = 0;
res |= !fwrite(&ih->biSize , sizeof(ih->biSize) , 1, output);
res |= !fwrite(&ih->biWidth , sizeof(ih->biWidth) , 1, output);
res |= !fwrite(&ih->biHeight , sizeof(ih->biHeight) , 1, output);
res |= !fwrite(&ih->biPlanes , sizeof(ih->biPlanes) , 1, output);
res |= !fwrite(&ih->biBitCount , sizeof(ih->biBitCount) , 1, output);
res |= !fwrite(&ih->biCompression , sizeof(ih->biCompression) , 1, output);
res |= !fwrite(&ih->biImageSize , sizeof(ih->biImageSize) , 1, output);
res |= !fwrite(&ih->biXPelsPerMeter, sizeof(ih->biXPelsPerMeter
In some of these functions I do need to allocate heap space with
malloc and actually write to disk with fwrite. These functions are doomed to fail from time-to-time (malloc can't find a big enough memory chunk to allocate and fwrite could fail as well).Now I'm unsure of how to handle failure situations reasonably. My current approach is to have a returned integer value indicate success.
```
uint8_t BMP32_write(FILE output, BMP32_FILE bmp32_file) {
uint8_t res = 0;
res |= BMP32_write_header(output, bmp32_file->bmph);
res |= BMP32_write_info_header(output, bmp32_file->bmpih);
res |= BMP32_write_rgbData(output, bmp32_file->bmpih->biWidth, bmp32_file->bmpih->biHeight, bmp32_file->rgbData);
return res;
}
static uint8_t BMP32_write_header(FILE output, BMP32_HEADER h) {
uint8_t res = 0;
res |= !fwrite(&h->bfType , sizeof(h->bfType) , 1, output);
res |= !fwrite(&h->bfSize , sizeof(h->bfSize) , 1, output);
res |= !fwrite(&h->bfReserved, sizeof(h->bfReserved), 1, output);
res |= !fwrite(&h->bfOffBits , sizeof(h->bfOffBits) , 1, output);
return res;
}
static uint8_t BMP32_write_info_header(FILE output, BMP32_INFO_HEADER ih) {
uint8_t res = 0;
res |= !fwrite(&ih->biSize , sizeof(ih->biSize) , 1, output);
res |= !fwrite(&ih->biWidth , sizeof(ih->biWidth) , 1, output);
res |= !fwrite(&ih->biHeight , sizeof(ih->biHeight) , 1, output);
res |= !fwrite(&ih->biPlanes , sizeof(ih->biPlanes) , 1, output);
res |= !fwrite(&ih->biBitCount , sizeof(ih->biBitCount) , 1, output);
res |= !fwrite(&ih->biCompression , sizeof(ih->biCompression) , 1, output);
res |= !fwrite(&ih->biImageSize , sizeof(ih->biImageSize) , 1, output);
res |= !fwrite(&ih->biXPelsPerMeter, sizeof(ih->biXPelsPerMeter
Solution
To answer you question on how to handle this, one needs to address why your current approach is good or not. And the main issue I have with your code is that if either
According to this documentation
Two alternate ways to handle these are:
Return at first fail
The easiest way to handle this, is to make your own
Skip later executions
An alternate version I've used in production code for a company is to have code which essentially behaves like the following:
If either
Expected return from
If your implementation of
If your implementation of
fwrite fails, you'll happily continue until all three helper methods have completed. This is not good behaviour, and might trigger bigger errors which might terminate your program, removing your chance of correcting the initial errors.According to this documentation
fwrite will return the number of bytes written, but you keep on or'ing the result as if you're expecting 0 to indicate an OK write. Either way your or-ing will possibly hide the initial error code. I.e if the first fail returns 1 and the second fails with 2 you get an error code of 3. Anyway the final error code is misleading as to why your code originally failed.Two alternate ways to handle these are:
- Return at first fail
- Skip later executions of
fwriteafter fail
Return at first fail
The easiest way to handle this, is to make your own
fwrite, which in addition to the normal parameters has a result flag variable initiated within BMP32_write, where the local variant never executes the fwrite if the flag variable is set. This removes the possibility for changing the error code, and re-executing the fwrite after failing.Skip later executions
An alternate version I've used in production code for a company is to have code which essentially behaves like the following:
res = res < 0 ? res : fwrite( ... );
res = res < 0 ? res : fwrite( ... );
res = res < 0 ? res : fwrite( ... );
/* Here I've used `res < 0` as the error situation, which might
need to be replaced with `res == 0` depending on your fwrite
implementation. */If either
fwrite fails, it sets a value for res and skips the other calls. This can be interleaved whenever you want to break out, i.e. it might be good to bail out BMP32_write if either of the three helper functions fails. This can be prettified using macros, if you want to.Expected return from
fwriteIf your implementation of
fwrite returns number of bytes written, you need to compare it each time you write to the file to check if an error situation has arisen or not (compare written bytes with original block size). This can be easily achieved if you write your own fwrite as suggested above.If your implementation of
fwrite does indeed return 0, you might have an issue if compiling your code on a different platform where they have a more standardized version of fwrite...Code Snippets
res = res < 0 ? res : fwrite( ... );
res = res < 0 ? res : fwrite( ... );
res = res < 0 ? res : fwrite( ... );
/* Here I've used `res < 0` as the error situation, which might
need to be replaced with `res == 0` depending on your fwrite
implementation. */Context
StackExchange Code Review Q#119703, answer score: 4
Revisions (0)
No revisions yet.