RFR: 8296812: sprintf is deprecated in Xcode 14

[Thomas Stuefe]

On Sun, 13 Nov 2022 22:55:52 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> Please don't add uses of `jio_snprintf` or `::snprintf` to hotspot. Use `os::snprintf`.
>> 
>> Regarding `jio_snprintf`, see https://bugs.openjdk.org/browse/JDK-8198918.
>> Regarding `os::snprintf` and `os::vsnprintf`, see https://bugs.openjdk.org/browse/JDK-8285506.
>> 
>> I think the only reason we haven't marked `::sprintf` and `::snprintf` forbidden
>> (FORBID_C_FUNCTION) is there are a lot of uses, and nobody has gotten around
>> to dealing with it.  `::snprintf` in the list of candidates for
>> https://bugs.openjdk.org/browse/JDK-8214976, some of which have already been
>> marked.  But I don't see new bugs for the as-yet unmarked ones.
>> 
>> As a general note, as a reviewer my preference is against non-trivial and
>> persnickety code changes that are scattered all over the code base. For
>> something like this I'd prefer multiple more bite-sized changes that were
>> dealing with specific uses.  I doubt everyone agrees with me though.
>
>> Please don't add uses of `jio_snprintf` or `::snprintf` to hotspot. Use `os::snprintf`.
> 
> Updated to use os::snprintf, except the files under adlc where the os::snptintf definition is not included.  The use of snprintf could be cleaned up with existing code in the future.
> 
>> 
>> Regarding `jio_snprintf`, see https://bugs.openjdk.org/browse/JDK-8198918. Regarding `os::snprintf` and `os::vsnprintf`, see https://bugs.openjdk.org/browse/JDK-8285506.
>> 
>> I think the only reason we haven't marked `::sprintf` and `::snprintf` forbidden (FORBID_C_FUNCTION) is there are a lot of uses, and nobody has gotten around to dealing with it. `::snprintf` in the list of candidates for https://bugs.openjdk.org/browse/JDK-8214976, some of which have already been marked. But I don't see new bugs for the as-yet unmarked ones.
>> 
>> As a general note, as a reviewer my preference is against non-trivial and persnickety code changes that are scattered all over the code base. For something like this I'd prefer multiple more bite-sized changes that were dealing with specific uses. I doubt everyone agrees with me though.
> 
> It makes sense to me.  I'd better focus on the building issue in this PR.
> 
> Thank you for the review!

Hi @XueleiFan and @kimbarrett ,

I agree to the change if we, as Kim suggested, add assertions for truncation where we use the return value of snprintf.

I am not fully happy with the solution though, since printing is notoriously runtime-data dependent and runtime data can change in release builds. So we could have truncation at a customer that we never see in our tests with debug builds.

But seeing that this patch takes so long now and blocks the MacOS build, I don't want to block it. We can improve the code in follow up RFEs. These places would be a lot simpler with stringStream.

Cheers, Thomas

-------------

PR: https://git.openjdk.org/jdk/pull/11115