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.

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

Fix the max memory request initial value in memory.c and comparison in jpeg.c #251

Merged
merged 2 commits into from
Jun 24, 2023

Conversation

drivron
Copy link
Contributor

@drivron drivron commented Jun 22, 2023

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practices as demonstrated in the repository.

Description

Avoids the error "Backing store not supported" when converting some jpeg (progressive in my case) on Windows (x86_64-w64-mingw32-gcc).

First, the initial value of max_memory_request in GetMaxMemoryRequest is inconsistent with the MagickMin return value. This was already fixed on ImageMagick 7:

I have not understood why MAGICK_SSIZE_MAX is preferred over MAGICK_SIZE_MAX.

Second, the comparison in coders/jpeg.c always returned true because neither ~0ULL nor MAGICK_SSIZE_MAX equals ~0UL on Windows when compiled with MinGW-w64, resulting in an invalid max memory size configuration in the jpeg library.

…oids the error "Backing store not supported" on Windows (x86_64-w64-mingw32-gcc)
@dlemstra
Copy link
Member

Thanks for fixing this in ImageMagick6. We just pushed the changes ImageMagick/ImageMagick@1242ca5 and ImageMagick/ImageMagick@a11148a to ImageMagick7. Can you also include that in this PR?

@drivron
Copy link
Contributor Author

drivron commented Jun 24, 2023

It turns out that SSIZE_MAX (i.e. MAGICK_SSIZE_MAX) is not equal to LONG_MAX on MinGW-w64. Why not use MAGICK_SSIZE_MAX in this comparison to avoid any doubt about this type representation?

@dlemstra
Copy link
Member

In the Windows x64 build LONG_MAX is a long and SSIZE_MAX is a long long. The new check makes sure we only set max_memory_to_use when the value is lower than the maximum value for a long.

@drivron
Copy link
Contributor Author

drivron commented Jun 24, 2023

Ok, I hadn't noticed the comparison operator change.
I updated the PR.

@dlemstra dlemstra merged commit 60a877e into ImageMagick:main Jun 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants