On Wed, Jun 3, 2026 at 8:00 AM Branko Čibej <[email protected]> wrote:
> On 3. 6. 2026 13:52, Nathan Hartman wrote: > > On Wed, Jun 3, 2026 at 7:03 AM <[email protected]> wrote: > >> Author: brane >> Date: Wed Jun 3 11:03:37 2026 >> New Revision: 1934920 >> >> Log: >> Use snprintf instead of sprintf if it's available. >> >> * CMakeLists.txt: Check for snprintf. >> * build/ac-macros/compiler.m4: Check for snprintf. >> >> * subversion/svn/filesize.c: Include svn_private_config.h. >> (format_size): Use snprintf if it's available. >> >> * subversion/tests/libsvn_subr/skel-test.c: Include svn_private_config.h. >> (put_explicit_length): Use snprintf if it's available. >> >> Modified: >> subversion/trunk/CMakeLists.txt >> subversion/trunk/build/ac-macros/compiler.m4 >> subversion/trunk/subversion/svn/filesize.c >> subversion/trunk/subversion/tests/libsvn_subr/skel-test.c >> >> Modified: subversion/trunk/CMakeLists.txt >> >> ============================================================================== >> --- subversion/trunk/CMakeLists.txt Wed Jun 3 10:44:38 2026 >> (r1934919) >> +++ subversion/trunk/CMakeLists.txt Wed Jun 3 11:03:37 2026 >> (r1934920) >> @@ -872,6 +872,8 @@ macro(autocheck_symbol_exists SYMBOL FIL >> endif() >> endmacro() >> >> +autocheck_symbol_exists("snprintf" "string.h" HAVE_SNPRINTF) >> + >> autocheck_include_files("elf.h" HAVE_ELF_H) >> autocheck_include_files("inttypes.h" HAVE_INTTYPES_H) >> autocheck_include_files("stdbool.h" HAVE_STDBOOL_H) >> >> Modified: subversion/trunk/build/ac-macros/compiler.m4 >> >> ============================================================================== >> --- subversion/trunk/build/ac-macros/compiler.m4 Wed Jun 3 >> 10:44:38 2026 (r1934919) >> +++ subversion/trunk/build/ac-macros/compiler.m4 Wed Jun 3 >> 11:03:37 2026 (r1934920) >> @@ -71,6 +71,9 @@ AC_DEFUN([SVN_CC_MODE_SETUP], >> ]) >> fi >> >> + dnl check for snprintf so that we can use it instead of sprintf. >> + AC_CHECK_FUNCS(snprintf) >> + >> CMODEFLAGS="$CFLAGS" >> CFLAGS="" >> SVN_DOT_CLANGD_CC([$CMODEFLAGS]) >> >> Modified: subversion/trunk/subversion/svn/filesize.c >> >> ============================================================================== >> --- subversion/trunk/subversion/svn/filesize.c Wed Jun 3 10:44:38 2026 >> (r1934919) >> +++ subversion/trunk/subversion/svn/filesize.c Wed Jun 3 11:03:37 2026 >> (r1934920) >> @@ -31,6 +31,7 @@ >> #include <apr_strings.h> >> >> #include "cl.h" >> +#include "svn_private_config.h" >> >> >> /*** Code. ***/ >> @@ -72,16 +73,6 @@ format_size(double human_readable_size, >> apr_size_t index, >> apr_pool_t *result_pool) >> { >> - /* Apple in its infinite wisdom has seen fit to deprecate sprintf() >> which >> - has been part of the C standard library since the K&R days and is >> not >> - deprecated in any version of the C standard. */ >> -#ifdef __APPLE__ >> -# if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 2) >> -# pragma GCC diagnostic push >> -# pragma GCC diagnostic ignored "-Wdeprecated-declarations" >> -# endif >> -#endif /* __APPLE__ */ >> - >> /* NOTE: We want to display a locale-specific decimal sepratator, but >> APR's formatter completely ignores the locale. So we use the >> good, old, standard, *dangerous* sprintf() to format the size. >> @@ -107,22 +98,26 @@ format_size(double human_readable_size, >> the absolute size is actually a single-digit number, because >> files can't have fractional byte sizes. */ >> if (absolute_human_readable_size >= 10) >> - sprintf(buffer, "%.0f", human_readable_size); >> + { >> +#if !HAVE_SNPRINTF >> + sprintf(buffer, "%.0f", human_readable_size); >> +#else >> + snprintf(buffer, sizeof(buffer), "%.0f", human_readable_size); >> +#endif >> + } >> else >> { >> double integral; >> const double frac = modf(absolute_human_readable_size, >> &integral); >> const int decimals = (index > 0 && (integral < 9 || frac <= >> .949999999)); >> +#if !HAVE_SNPRINTF >> sprintf(buffer, "%.*f", decimals, human_readable_size); >> +#else >> + snprintf(buffer, sizeof(buffer), "%.*f", decimals, >> human_readable_size); >> +#endif >> } >> >> return apr_pstrcat(result_pool, buffer, suffix, SVN_VA_NULL); >> - >> -#ifdef __APPLE__ >> -# if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 2) >> -# pragma GCC diagnostic pop >> -# endif >> -#endif /* __APPLE__ */ >> } >> >> >> >> Modified: subversion/trunk/subversion/tests/libsvn_subr/skel-test.c >> >> ============================================================================== >> --- subversion/trunk/subversion/tests/libsvn_subr/skel-test.c Wed Jun >> 3 10:44:38 2026 (r1934919) >> +++ subversion/trunk/subversion/tests/libsvn_subr/skel-test.c Wed Jun >> 3 11:03:37 2026 (r1934920) >> @@ -33,6 +33,7 @@ >> >> #include "../svn_test.h" >> #include "../svn_test_fs.h" >> +#include "svn_private_config.h" >> >> >> /* Some utility functions. */ >> @@ -307,24 +308,19 @@ put_explicit_length(svn_stringbuf_t *str >> apr_size_t len, >> char sep) >> { >> - /* Apple in its infinite wisdom has seen fit to deprecate sprintf() >> which >> - has been part of the C standard library since the K&R days and is >> not >> - deprecated in any version of the C standard. */ >> -#ifdef __APPLE__ >> -# if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 2) >> -# pragma GCC diagnostic push >> -# pragma GCC diagnostic ignored "-Wdeprecated-declarations" >> -# endif >> -#endif /* __APPLE__ */ >> - >> - char *buf = malloc(len + 100); >> + const apr_size_t alloc_len = len + 100; >> + char *buf = malloc(alloc_len); >> apr_size_t length_len; >> >> if (! skel_is_space(sep)) >> abort(); >> >> /* Generate the length and separator character. */ >> +#if !HAVE_SNPRINTF >> sprintf(buf, "%"APR_SIZE_T_FMT"%c", len, sep); >> +#else >> + snprintf(buf, alloc_len, "%"APR_SIZE_T_FMT"%c", len, sep); >> +#endif >> length_len = strlen(buf); >> >> /* Copy in the real data (which may contain nulls). */ >> @@ -332,12 +328,6 @@ put_explicit_length(svn_stringbuf_t *str >> >> svn_stringbuf_appendbytes(str, buf, length_len + len); >> free(buf); >> - >> -#ifdef __APPLE__ >> -# if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 2) >> -# pragma GCC diagnostic pop >> -# endif >> -#endif /* __APPLE__ */ >> } >> >> >> >> > Out of curiosity, what made you change your mind? > > (I'm glad to see it, because I think this is simpler and more future-proof > than checking for specific GNUC version values on Apple machines. It's also > much more self-documenting.) > > > > Basically, 1.15.x vs trunk. It's appropriate to silence the warning in > 1.15.x, but not to push essentially untested code into a new release. This > change will have time to settle on trunk for 1.16. I don't intend to > propose this commit for backport. > Sounds perfectly sensible to me. +1 to this plan. I still have a very concrete opinion about deprecating standard functions, > that has not and will not change. People who want to "fix" the C library > should put the work in, get on the appropriate ISO committee, and see how > they like that. It's easier to get specific warnings into gcc/clang. > Well, they'd have to get on the C89 committee or, better yet, go farther back and have a talk with K&R. Getting a working time machine is trivial and left as an exercise. Nathan
