RFR: 8296812: sprintf is deprecated in Xcode 14 [v6]

[Xue-Lei Andrew Fan]

On Tue, 15 Nov 2022 05:52:18 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> Xue-Lei Andrew Fan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   delete swp file
>
> src/hotspot/cpu/aarch64/vm_version_aarch64.cpp line 226:
> 
>> 224:   char buf[512];
>> 225:   os::snprintf(buf, sizeof(buf), "0x%02x:0x%x:0x%03x:%d", _cpu, _variant, _model, _revision);
>> 226:   if (_model2) os::snprintf(buf+strlen(buf), sizeof(buf) - strlen(buf), "(0x%03x)", _model2);
> 
> Here - and in several other places, where we construct a string from multiple parts - the code would be a simpler with `stringStream`:
> 
> 
> char buf[512];
> stringStream ss(buf, sizeof(buf));
> ss.print("0x%02x:0x%x:0x%03x:%d", _cpu, _variant, _model, _revision);
> if (_model2) ss.print("(0x%03x)", _model2);
> _features_string = os::strdup(buf);
> 
> or, using `stringStream`s internal buffer:
> 
> 
> stringStream ss;
> ss.print("0x%02x:0x%x:0x%03x:%d", _cpu, _variant, _model, _revision);
> if (_model2) ss.print("(0x%03x)", _model2);
> _features_string = ss.base();
> 
> 
> No manual offset counting required.
> 
> I leave it up to you if you do it that way. The code here is correct as it is.

Glad to know that `stringStream` could make this block safer and easier.  I learned a lot from the reviewers of this PR.  It looks like we can also use stringStream for other files touched in this PR.  I would like to keep the update focusing on the simple replacing of `sprintf`.  I may file a new PR for the improvement by using `stringStream` shortly after.

> src/hotspot/share/classfile/javaClasses.cpp line 2562:
> 
>> 2560:       CompiledMethod* nm = method->code();
>> 2561:       if (WizardMode && nm != NULL) {
>> 2562:         os::snprintf(buf + buf_off, buf_size - buf_off, "(nmethod " INTPTR_FORMAT ")", (intptr_t)nm);
> 
> I think you should update `buf_off` here, because now you overwrite the last text part. Weird that no test caught that.
> 
> All this code here in javaClasses.cpp would benefit from using stringStream.

Oooops!  Good catch.  `buf_off` is updated in the new commit.  I think `stringStream` could be a better solution.  I will do it in a follow-up PR.

> src/hotspot/share/utilities/utf8.cpp line 521:
> 
>> 519:     } else {
>> 520:       if (p + 6 >= end) break;      // string is truncated
>> 521:       os::snprintf(p, 7, "\\u%04x", c);
> 
> This should be 6, or? We have 6 characters left before end, assuming end is exclusive.
> 
> Also, maybe use a named constant?

If 6 is used, there is a output truncated warning and only 5 characters are filed actually.  The terminating null/zero is counted in, I think.  To make it easier to read, I added a comment.

> src/java.desktop/macosx/native/libjsound/PLATFORM_API_MacOSX_Ports.cpp line 638:
> 
>> 636:                         return;
>> 637:                     }
>> 638:                     snprintf(channelName, 16, "Ch %d", ch);
> 
> Can we use a constant here instead of literal 16?

To be honest, I don't know the logic of the code yet.  I'm not sure how to name the literal 16 yet.

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

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