Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rework libpng error handling to catch version mismatch errors as well #134

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Rework libpng error handling to catch version mismatch errors as well #134

wants to merge 4 commits into from

Commits on Nov 26, 2022

  1. Png{Importer,ImageConverter}: rework libpng version mismatch handling. 

    Before 7e55228, there was an assert
where even patch version jump caused the plugin to abort. Which happens
pretty regularly, last with 1.6.38 -> 39, and is nothing that should be
treated as a critical failure (I wonder what "libpng tutorial" was that
taken from, sigh). The above-referenced commit removed this assert in
order to unblock users, however it meant that a hypotetical upgrade from
1.6.x to 1.7 would cause the png_create_{read,write}_struct() to print
for example

    libpng warning: Application built with libpng-1.7.0 but running with 1.6.39

directly to the stdout (thus, impossible to see on e.g. Android), and
then the plugin immediately does on CORRADE_INTERNAL_ASSERT(png.file).
This commit attempts to fix it properly.

The png_create_{read,write}_struct() API accepts error/warning callbacks
on its own, so supplying them there instead of in png_set_error_fn()
allows it to direct the version mismatch error to us. Which is how it
should have been done in the first place (and again, I have no idea why
the *official* libpng documentation doesn't show that, FFS).

Unfortunately, libpng directs the major/minor version mismatch *fatal
error* to the warning callback, which is extremely confusing, and
there's no way to detect and fixup the case apart from comparing the
string itself. Sigh.

Co-authored-by: Squareys <[email protected]>
    @mosra @Squareys
    mosra and Squareys committed Nov 26, 2022
    Configuration menu
    Copy the full SHA
    dbc7a7a View commit details
    Browse the repository at this point in the history

  2. [wip] Png{Importer,ImageConverter}: attempt to work around longjmp is… 

    …sues.

This makes the code no longer use png_jmpbuf() in an attempt to fix the
clang-cl error, as I suspected the misbehavior was due to longjmp being
possibly called even before setjmp() is called in our code.

But, going through libpng sources, it seems that for the version
mismatch check the png_create_*_struct() itself calls setjmp(), so there
should be no problem, and this whole extra complication is not needed.

And, indeed, it has no effect on the behavior -- the clang-cl build
still crashes or asserts due to some stack corruption.
    @mosra
    mosra committed Nov 26, 2022
    Configuration menu
    Copy the full SHA
    c5a9298 View commit details
    Browse the repository at this point in the history

  3. [wip]

    @mosra
    mosra committed Nov 26, 2022
    Configuration menu
    Copy the full SHA
    5929834 View commit details
    Browse the repository at this point in the history

  4. [wip] clang-cl build of libpng?

    @mosra
    mosra committed Nov 26, 2022
    Configuration menu
    Copy the full SHA
    00e7843 View commit details
    Browse the repository at this point in the history