On Tue, 22 Nov 2022 08:02:51 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
> Given all the near-duplicated checking of os::snprintf results, I think there
> is a place for a helper function to package this up. Maybe something like
>
> ```
> // in class os
> // Performs snprintf and asserts the result is non-negative (so there was not
> // an encoding error) and that the output was not truncated.
> static int snprintf_checked(char* buf, size_t len, const char* fmt, ...)
> ATTRIBUTE_PRINTF(3, 4);
>
> // in runtime/os.cpp
> int os::snprintf_checked(char* buf, size_t len, const char* fmt, ...) {
> va_list args;
> va_start(args, fmt);
> int result = os::vsnprintf(buf, len, fmt, args);
> va_end(args);
> assert(result >= 0, "os::snprintf error");
> assert(static_cast<size_t>(result) < size, "os::snprintf truncated");
> return result;
> }
> ```
>
> (I keep waffling over whether the truncation check should be an assert or a
> guarantee.)
>
> I've not yet gone through all the changes yet to consider which should do
> that checking and which should do something different, such as permitting
> truncation.
>
> I'm not wedded to that name; indeed, I don't like it that much, as it's kind
> of inconveniently long. There's a temptation to have os::snprintf forbid
> truncation and a different function that allows it, but that would require
> careful auditing of all pre-existing uses of os::snprintf too, so no.
How about renaming the existing os::snprintf to something like
os::snprintf_unchecked, make os::snprintf the checked version, then, in
separate RFEs, revert existing uses to the new API. When all uses of
os::snprintf_unchecked are cleared up, remove it.
That would make it possible to revert piecemeal while not racing with new uses
of os::snprintf, since new callers will use the new checking API automatically.
-------------
PR: https://git.openjdk.org/jdk/pull/11115
