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

openjp2/j2k: replace sprintf calls with snprintf #1450

Merged
merged 2 commits into from
Mar 9, 2023

Conversation

markmentovai
Copy link
Contributor

@markmentovai markmentovai commented Nov 7, 2022

This makes it possible to build j2k.c without warnings using the macOS 13 SDK. Calls to sprintf are replaced with snprintf, passing appropriate buffer sizes.

It doesn’t appear that any of the changed uses of sprintf were actually unsafe, so no behavior change is expected aside from SDK compatibility.

The macOS 13 SDK deprecates sprintf as it’s difficult to use safely. The deprecation warning message is visible when building C++, but it is not normally visible when building plain C code due to a quirk in how sprintf is declared in the SDK. However, the deprecation message is visible when building plain C under Address Sanitizer (-fsanitize=address). This discrepancy was discovered at https://crbug.com/1381706 and reported to Apple with a copy at https://openradar.appspot.com/FB11761475.

The macOS 13 SDK is packaged in Xcode 14.1, released on 2022-11-01. This also affects the iOS 16 SDK and other 2022-era Apple OS SDKs packaged in Xcode 14.0, released on 2022-09-12.

j2k.c is visible to the Chromium build via PDFium, and this change is needed to allow Chromium to move forward to the macOS 13 SDK.

This change is limited to src/lib/openjp2. Other uses of sprintf were found throughout openjpeg.

ibaoger pushed a commit to ibaoger/pdfium that referenced this pull request Nov 7, 2022
This makes it possible to build PDFium without warnings using the macOS
13 SDK. Calls to sprintf are replaced with snprintf, passing appropriate
buffer sizes.

It doesn’t appear that any of the changed uses of sprintf were actually
unsafe, so no behavior change is expected aside from SDK compatibility.

The macOS 13 SDK deprecates sprintf as it’s difficult to use safely. The
deprecation warning message is visible when building C++, but it is not
normally visible when building plain C code due to a quirk in how
sprintf is declared in the SDK. However, the deprecation message is
visible when building plain C under Address Sanitizer
(-fsanitize=address). This discrepancy was discovered at
https://crbug.com/1381706 and reported to Apple with a copy at
https://openradar.appspot.com/FB11761475.

The macOS 13 SDK is packaged in Xcode 14.1, released on 2022-11-01. This
also affects the iOS 16 SDK and other 2022-era Apple OS SDKs packaged in
Xcode 14.0, released on 2022-09-12.

This change affects libopenjpeg and libtiff. The patches here were
submitted upstream at uclouvain/openjpeg#1450
and https://gitlab.com/libtiff/libtiff/-/merge_requests/408.

Bug: chromium:1381706
Change-Id: I92ff8f897fe63b3dee14301cd15c791e0d4fb8cf
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/100790
Commit-Queue: Lei Zhang <thestig@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
@rouault rouault closed this Mar 7, 2023
@rouault rouault reopened this Mar 7, 2023
rouault and others added 2 commits March 7, 2023 16:47
This makes it possible to build j2k.c without warnings using the macOS
13 SDK. Calls to sprintf are replaced with snprintf, passing appropriate
buffer sizes.

It doesn’t appear that any of the changed uses of sprintf were actually
unsafe, so no behavior change is expected aside from SDK compatibility.

The macOS 13 SDK deprecates sprintf as it’s difficult to use safely. The
deprecation warning message is visible when building C++, but it is not
normally visible when building plain C code due to a quirk in how
sprintf is declared in the SDK. However, the deprecation message is
visible when building plain C under Address Sanitizer
(-fsanitize=address). This discrepancy was discovered at
https://crbug.com/1381706 and reported to Apple with a copy at
https://openradar.appspot.com/FB11761475.

The macOS 13 SDK is packaged in Xcode 14.1, released on 2022-11-01. This
also affects the iOS 16 SDK and other 2022-era Apple OS SDKs packaged in
Xcode 14.0, released on 2022-09-12.

j2k.c is visible to the Chromium build via PDFium, and this change is
needed to allow Chromium to move forward to the macOS 13 SDK.

This change is limited to src/lib/openjp2. Other uses of sprintf were
found throughout openjpeg.
@rouault rouault merged commit fbdf28a into uclouvain:master Mar 9, 2023
mayeut added a commit to mayeut/openjpeg that referenced this pull request Sep 24, 2023
With uclouvain#1450 which goes with 480cc9d "Remove support for non-C99 compilers (like VS2010) that don't support snprintf()",
support for MSVC versions prior to vs2015 is dropped: https://stackoverflow.com/questions/2915672/snprintf-and-visual-studio-2010

This means that all supported MSVC versions do have `stdint.h` & `inttypes.h` now.
For non windows platforms, those headers were already mandatory.

Make them mandatory for all builds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants