Repository navigation
[R] Switch sprintf to snprintf #3724
Description
From @rcurtin regarding the 4.4.0 R build on matrix, we have R CMD check returning:
* checking compiled code ... WARNING
File ‘mlpack/libs/mlpack.so’:
Found ‘__sprintf_chk’, possibly from ‘sprintf’ (C)
Object: ‘image_converter.o’
The quick fix here is to switch sprintf() use cases to snprintf() in the offending stb_image_write.h library to ensure a maximum number of bytes to be used in the buffer declared.
/cc @eddelbuettel
Activity
eddelbuettel commentedon May 30, 2024
Good catch. This is checked for.
conradsnicta commentedon May 31, 2024
@coatless @eddelbuettel Looks like the problem is in the STB dependency. The simplest solution might be to build the R mlpack package without STB image support.
Suggest to remove all stb related packages from the build environment (eg. libstb-dev on Ubuntu/Debian), or otherwise modify mlpack build scripts (CMakeLists.txt) and/or configuration to explicitly disable stb.
It's not much of a loss, as reading/writing files is typically handled by users at the R level, not at the C++ level. The main focus of mlpack is machine learning anyway, not image loading.
Alternatively, you can send a PR to the STB folks fixing this issue (https://github.com/nothings/stb). It may take a while to propagate this downstream (ie. distro packages), so it won't fix the present R packaging problem any time soon.
Ryan is time constrained, so anything that reduces the load is useful.
rcurtin commentedon May 31, 2024
You are right that
image_convertermay not be useful to R developers, but honestly I think the easier bet is to just to bundle a manually-patched STB version (it's just two header files) with the R tarball. (In fact distributingimage_converterwithout it is unsafe, as the user's system is not guaranteed to havestb_image.handstb_image_write.hinstalled.) Thesprintf()tosnprintf()change should be trivial anyway, and would be a worthwhile thing to patch upstream.(I am time-constrained, but if someone else doesn't get to this before me I'll do it. I don't think it'll take very long.)
eddelbuettel commentedon May 31, 2024
What @rcurtin suggests is what we commonly do to please CRAN. (I.e. we have edit or ifdef out some things).
What @conradsnicta refers to also works because will not check externally linked code: only the shared library the package produces.