OpenImageIO
OpenImageIO copied to clipboard
AcademySoftwareFoundation
ImageInput: Errors that cause open() to fail did not close the file handle.
ImageInput: Errors that cause open() to fail did not close the file handle.
This was reported as a problem with DDS, but after being urged to poke around at all the ImageInput subclasses, it was widespread that errors in the open() methods did not always release open file handles.
As a practical matter, this is not super important, because in all cases, destroying the ImageInput -- expected to happen by the caller if open fails -- would also cause all the handles to be closed. But I agree that it's nicer to do so immediately upon realizing it's an unreadable file.
I added more tests to imageinout_test, where for each format we create a bogus file and check that the format's valid_file() fails properly, and also check that valid_file() succeeds properly for a valid file. (And, oops, in the process detected that SGI format did not work properly because of failure to deal with endianness in valid_file, though it did so for open.)
This is one of those times when you think you're pulling just one little loose thread on the sweater, and next thing you know it exposes a bunch of issues spread all over that you never knew were a problem. But that's how software gets better.
Fixes #2240
Pushed a big update. Now this is not just specific to DDS, but audits and fixes all ImageInput subclasses that exhibited the problem.
Yet another update. Added more imageinout_test tests to ensure that valid_file properly works for valid files and fails for bogus files. And that uncovered another interesting bug. But I think I have everything now.
The new description is:
ImageInput: Errors that cause open() to fail did not close the file handle.
This was reported as a problem with DDS, but after being urged to poke around at all the ImageInput subclasses, it was widespread that errors in the open() methods did not always release open file handles.
As a practical matter, this is not super important, because in all cases, destroying the ImageInput -- expected to happen by the caller if open fails -- would also cause all the handles to be closed. But I agree that it's nicer to do so immediately upon realizing it's an unreadable file.
I added more tests to imageinout_test, where for each format we create a bogus file and check that the format's valid_file() fails properly, and also check that valid_file() succeeds properly for a valid file. (And, oops, in the process detected that SGI format did not work properly because of failure to deal with endianness in valid_file, though it did so for open.)
This is one of those times when you think you're pulling just one little loose thread on the sweater, and next thing you know it exposes a bunch of issues spread all over that you never knew were a problem. But that's how software gets better.
Fixes #2240
I'm not sure why this one languished and fell through the cracks. I will rebase and if it passes all tests, will merge it.
Closing. I accidentally pushed the wrong thing over this branch. It couldn't have been important anyway if I let this languish for 3 years.