RFR: 8213622 - Windows VS2013 build failure - "'snprintf': identifier not found"

[JC Beyler]

Hi all,

I created this change instead:
http://cr.openjdk.java.net/~jcbeyler/8213622/webrev.00/

It removes the sprintf calls for strlen and strncpy calls. I checked that
the code works if I force an error on GetObjectClass for example:

FATAL ERROR in native method: GetObjectClass : Return is NULL
at
nsk.share.gc.lock.CriticalSectionTimedLocker.criticalSection(CriticalSectionTimedLocker.java:47)
at
nsk.share.gc.lock.CriticalSectionLocker$1.run(CriticalSectionLocker.java:56)
at nsk.share.gc.lock.jniref.JNILocalRefLocker.criticalNative(Native Method)
at
nsk.share.gc.lock.jniref.JNILocalRefLocker.criticalSection(JNILocalRefLocker.java:44)
at
nsk.share.gc.lock.CriticalSectionTimedLocker.criticalSection(CriticalSectionTimedLocker.java:47)
at
nsk.share.gc.lock.CriticalSectionLocker$1.run(CriticalSectionLocker.java:56)
at java.lang.Thread.run(java.base at 12-internal/Thread.java:835)

I'll pass it through the submit repo if this looks like a better fix.

Let me know what you think,
Jc

On Sun, Nov 11, 2018 at 10:38 PM JC Beyler <jcbeyler at google.com> wrote:

> Hi all,
>
> If I've caught up with the conversation, it sounds like:
>   - My code breaks VS2013 build but that soon that won't be supported
>   - We can't just do a renaming macro due to my use of sprintf with an
> empty buffer
>       - So we need to either do what was suggested by Kim:
> "That first snprintf call could be rewritten using several calls to
> strlen to calculate the needed size in some different manner. The
> later call could also be rewritten to use several calls to strcpy
> rather than snprintf."
>          Or something to that effect
>
> I can work on a fix that will handle this if we want, I'm just not sure if
> that's the path that is being recommended here though. It seems that in
> this particular case, it is not really hard to fix the code for now; the
> day VS2013 is no longer build-able by other pieces of code we can come back
> to this solution. To be honest, the reason this is like this is due to not
> being able to have C++ strings when I tried initially. The day we can have
> those, then this code can become even simpler.
>
> Let me know what you think and would prefer,
> Jc
>
> On Sun, Nov 11, 2018 at 9:01 PM David Holmes <david.holmes at oracle.com>
> wrote:
>
>> Hi Michal,
>>
>> On 12/11/2018 2:18 PM, Michal Vala wrote:
>> >
>> >
>> > On 11/10/18 12:07 AM, David Holmes wrote:
>> >> cc'ing JC Beyler as this was his code.
>> >>
>> >> On 10/11/2018 4:35 AM, Kim Barrett wrote:
>> >>>> On Nov 9, 2018, at 11:43 AM, Michal Vala <mvala at redhat.com> wrote:
>> >>>>
>> >>>> Hi,
>> >>>>
>> >>>> please review following patch fixing build failure on Windows with
>> >>>> VS2013 toolchain.
>> >>
>> >> Not sure we're still supporting VS2013 ... ??
>> >
>> > Minimum accepted by configure is 2010. 2013 is 2nd in priorities after
>> > 2017. It has `VS_SUPPORTED_2013=false` though, but why not keep it
>> > buildable?
>>
>> That depends on how painful it is to achieve that. As Kim has explained
>> the suggested fix may allow building but it isn't correct.
>>
>> And we may not be able to keep this ability to build with VS2013 going
>> forward.
>>
>> Thanks,
>> David
>>
>>
>> >>
>> >>>>
>> >>>> http://cr.openjdk.java.net/~mvala/jdk/jdk/JDK-8213622/webrev.00/
>> >>>
>> >>> The failure is in a hotspot test.  It should be using os::snprintf.
>> >>
>> >> Tests don't have access to os::snprintf. The test is just C++ code.
>> >>
>> >> Cheers,
>> >> David
>> >>
>> >>
>> >
>>
>
>
> --
>
> Thanks,
> Jc
>


-- 

Thanks,
Jc