RFR (L): 8062370: Various minor code improvements

[Lindenmaier, Goetz]

Hi David, 

yes, windows does not null terminate if there is an overflow.
Obviously there are overflows, and they now see one less 
character.  I think this should be fixed where jio_vsnprintf 
is called.  Having non-null terminated strings isn't nice.

But for now I will roll back this single change.  I'll send a RFR soon.

Where did you see the problem?

Best regards,
  Goetz.




-----Original Message-----
From: David Holmes [mailto:david.holmes at oracle.com] 
Sent: Donnerstag, 6. November 2014 11:30
To: Lindenmaier, Goetz; hotspot-dev at openjdk.java.net
Cc: Markus Grönlund
Subject: Re: RFR (L): 8062370: Various minor code improvements

On 6/11/2014 8:17 PM, Lindenmaier, Goetz wrote:
> Thanks David, I'll have a look.

It seems that windows vsnprintf may not null-terminate the string - 
which I think is what your patch was trying to address. But if we have 
existing code that works with that then the fix is now overwriting the 
last character. I can't quite see how to handle this in a cross platform 
manner, but in the immediate term we should probably revert that part of 
the changeset.

David

> Best regards,
>    Goetz.
>
> -----Original Message-----
> From: David Holmes [mailto:david.holmes at oracle.com]
> Sent: Donnerstag, 6. November 2014 11:09
> To: Lindenmaier, Goetz; hotspot-dev at openjdk.java.net
> Cc: Markus Grönlund
> Subject: Re: RFR (L): 8062370: Various minor code improvements
>
> Hi Goetz,
>
> This change has introduced a bug:
>
> -  return vsnprintf(str, count, fmt, args);
> +
> +  int result = vsnprintf(str, count, fmt, args);
> +  if ((result > 0 && (size_t)result >= count) || result == -1) {
> +    str[count - 1] = '\0';
> +    result = -1;
> +  }
> +
> +  return result;
>
> some strings are getting their last character truncated on Windows.
>
> David
>
> On 5/11/2014 6:16 PM, Lindenmaier, Goetz wrote:
>> Hi David,
>>
>> thanks for looking at the change!  I fixed the issue in a new
>> webrev:
>> http://cr.openjdk.java.net/~goetz/webrevs/8062370/webrev.01/
>>
>> Best regards,
>>     Goetz.
>>
>> -----Original Message-----
>> From: David Holmes [mailto:david.holmes at oracle.com]
>> Sent: Mittwoch, 5. November 2014 02:49
>> To: Lindenmaier, Goetz; hotspot-dev at openjdk.java.net
>> Subject: Re: RFR (L): 8062370: Various minor code improvements
>>
>> Hi Goetz,
>>
>> The only issue I see is in:
>>
>> src/share/vm/runtime/globals.cpp
>>
>> where you replaced NEW_C_HEAP_ARRAY with os::strdup. To keep the "abort
>> on OOM" semantics of NEW_C_HEAP_ARRAY you need to use os::strdup_check_oom.
>>
>> Thanks,
>> David
>>
>> On 30/10/2014 6:28 PM, Lindenmaier, Goetz wrote:
>>> Hi,
>>>
>>> this change contains a row of minor code improvements we did to fulfil
>>> our internal quality requirements.  We would like to share these with
>>> openJDK.
>>>
>>> Please review and test this change.  I please need a sponsor.
>>> http://cr.openjdk.java.net/~goetz/webrevs/8062370/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8062370
>>>
>>> We tested this on windows 64, linux x86_64, mac, solaris sparc 32+64 bit and,
>>> of course, the ppc platforms.
>>>
>>>
>>> Some details:
>>>
>>> CONST64(0x8000000000000000) is wrong, as 0x8... is positive, and thus not representable as i64 what is used in the CONST64 macro. This change adapts UCONST64 to use ui64, and the usages of these macros where necessary.
>>>
>>> We add some more strncpy uses.  Also, we fix strncpy on windows.  There, strncpy does not write a \0 into the last byte if the copied string is too long.
>>>
>>> We add some missing memory frees and some closing of files.
>>>
>>> jio_vsnprintf() works differently on windows and linux. This change adapts this to show the same behaviour on all platforms. See java.cpp.
>>>
>>> Best regards,
>>>
>>>      Goetz
>>>
>>>
>>>
>>>