phoboslab / qoi

The “Quite OK Image Format” for fast, lossless image compression
MIT License
6.83k stars 328 forks source link

More strict stdio error checking #268

Closed N-R-K closed 1 year ago

N-R-K commented 1 year ago

Error check qoi_write() more strictly

simply checking the return value of fwrite() wouldn't be enough since stdio is typically buffered. and so force a flush and check for errors via ferror().

Error check qoi_read() more strictly

fseek is not guaranteed to succeed and can fail, e.g when trying to seek a pipe. also check for fread errors too.

phoboslab commented 1 year ago

An extra check if fwrite() wrote the whole file is certainly a good idea. I guess qoi_write() should return 0 if fwrite() didn't write the whole size.

I'm not a fan of calling fflush() here. If I write files at 30fps (e.g. as a recording from a game) I explicitly don't want to force a flush. If you need this guarantee, there's always the option of calling qoi_encode() and do the writing yourself.

The check for a successful fseek() and bytes_read are fine, but the assignment in the if() statement is a bit too clever for my taste. Also, wouldn't the ftell() after the first failed fseek() return 0? So we'd only need the to check if the second fseek() was successful!?

N-R-K commented 1 year ago

I'm not a fan of calling fflush() here. If I write files at 30fps (e.g. as a recording from a game) I explicitly don't want to force a flush.

fclose() already calls fflush (see fclose(3)). But because fclose() also closes the FILE stream, we cannot use ferror on it anymore. Hence why the explicit flush.

but the assignment in the if() statement is a bit too clever for my taste.

I'll try to refactor it.

Also, wouldn't the ftell() after the first failed fseek() return 0? So we'd only need the to check if the second fseek() was successful!?

Hmm, I think you're right. That simplifies things a bit.