On Tue, 19 Aug 2025 22:02:30 GMT, David Holmes <dhol...@openjdk.org> wrote:
> This is a proposal to standardize on the use of `os::snprintf` and > `os::snprintf`_checked across the hotspot code base, and to disallow use of > the C library variants. (It does not touch use of `jio_printf` at all.) > > From: https://bugs.openjdk.org/browse/JDK-8347707 > > The platform `snprintf/vsnprintf` returns -1 on error, else if the buffer is > large enough returns the number of bytes written (excluding the null byte), > else (buffer is too small) the number of characters (excluding the > terminating null byte) which would have been written to the final string if > enough space had been available. Thus, a return value of size or more means > that the output was truncated. > > To provide a consistent approach to error handling and truncation management, > we provide `os::xxx` wrapper functions as described below and forbid the use > of the library `::vsnprintf` and `::snprintf`. > > The potential errors are, generally speaking, not something we should > encounter in our own well-written code: > > - encoding error: not applicable as we are not using extended character sets > - invalid parameters (null buffers, specifying a limit > size of the buffer > [Windows], things of this nature) > - mal-formed formatting directives > - overflow error (POSIX) if the required buffer size exceeds INT_MAX (as we > return `int`). > > As these should simply never occur, we handle the checks for -1 at the > lowest-level (`os::vsnprintf`) with an assertion, and accompanying > precondition assertions. > > The potential clients of this API then fall into a number of camps: > > 1. Those who have sized their buffer correctly, don't need the return value > for subsequent use, and for whom truncation (if it were possible) would be a > programming error. > > For these clients we have `void os::snprintf_checked` - which returns nothing > and asserts on truncation. > > 2. Those who have sized their buffer correctly, but do need the return value > for subsequent operations (e.g. chains of `snprintf` where you advance the > buffer pointer based on previous writes), but again for whom truncation > should never happen. > > For these clients we have `os::snprintf`, but they have to add their own > assertion for no truncation. > > 3. Those who present a buffer but know that truncation is a possibility, but > don't need to do anything about it themselves, and for whom the return value > is of no use. > > These clients also use `os::snprintf_checked`. The truncation assertion can > be useful for guiding buffer sizing decisions, but in product mode truncation > is not an error. > ... Could we map snprintf to os::snprintf_checked with a macro or some kind of namespace magic? It would reduce the number of files changed and catch any new cases of snprintf that might get accidentally added in the future. ------------- PR Comment: https://git.openjdk.org/jdk/pull/26849#issuecomment-3208122700
