RFR: 8038296 sun/tools/jinfo/Basic.sh: java.io.IOException: Command failed in target VM

Staffan Larsen staffan.larsen at oracle.com
Mon Apr 7 06:31:08 UTC 2014


Could I have an official Review of this change?

Thanks,
/Staffan

On 3 apr 2014, at 11:31, Staffan Larsen <staffan.larsen at oracle.com> wrote:

> Thanks Serguei,
> 
> I don’t think it is necessary to initialize ‘end’ since strtol will always set it.
> 
> I still need an official Reviewer for this change.
> 
> Thanks,
> /Staffan
> 
> On 28 mar 2014, at 07:21, serguei.spitsyn at oracle.com wrote:
> 
>> On 3/27/14 12:55 AM, Staffan Larsen wrote:
>>> Here is an updated webrev which incorporates Dmitry’s feedback:
>>> 
>>> http://cr.openjdk.java.net/~sla/8038296/webrev.01/
>> 
>> 
>> It looks good.
>> The only suggestion is to initialize the 'end':
>> char* end;
>> 
>> Not sure what value is better to use for initialization.
>> Probably, end = probe would work Ok.
>> 
>> Thanks,
>> Serguei
>>> 
>>> Thanks,
>>> /Staffan
>>> 
>>> On 25 mar 2014, at 19:36, Dmitry Samersoff <dmitry.samersoff at oracle.com> wrote:
>>> 
>>>> Staffan,
>>>> 
>>>>> Yes, that will find problems when trying to convert something like
>>>>> ‘bla’. It will not capture the case where the input string is a too
>>>>> large (or small) value (value < LONG_MIN or > LONG_MAX). To capture
>>>>> both cases it looks like we need something like:
>>>>> errno = 0;
>>>>> char* end;
>>>>> int probe_typess = (int) strtol(probe, &end, 10);
>>>>> if (end == probe || errno) {
>>>>>   return JNI_ERR;
>>>>> }
>>>> As probe_typess is positive and you are converting long to int
>>>> It's be better to check value boundaries explicitly:
>>>> 
>>>>  char* end;
>>>>  long ptl = strtol(probe, &end, 10);
>>>>  if (end == probe || ptl < 0 || ptl > MAX_INT) {
>>>>    return JNI_ERR;
>>>>  }
>>>> 
>>>>  int probe_typess = (int) ptl;
>>>> 
>>>> 
>>>> -Dmitry
>>>> 
>>>> 
>>>> On 2014-03-25 17:35, Staffan Larsen wrote:
>>>>> On 25 mar 2014, at 13:46, Dmitry Samersoff
>>>>> <dmitry.samersoff at oracle.com> wrote:
>>>>> 
>>>>>> Staffan,
>>>>>> 
>>>>>> strtol() sets errno in two cases -
>>>>>> 
>>>>>> ERANGE if supplied value is less then LONG_MIN or greater than
>>>>>> LONG_MAX EINVAL if supplied base is not supported.
>>>>>> 
>>>>>> if you pass probe == 'bla', strtol just return 0 and errno will not
>>>>>> be set. So I'm not sure that check for errno has any value here.
>>>>>> 
>>>>>> One of possible way to check that supplied value is convertible to
>>>>>> long is
>>>>>> 
>>>>>> char *eptr = probe; strtol(probe, (char **)&eptr, 10); if (eptr ==
>>>>>> probe) { // we can't convert supplied value return JNI_ERR; }
>>>>> Yes, that will find problems when trying to convert something like
>>>>> ‘bla’. It will not capture the case where the input string is a too
>>>>> large (or small) value (value < LONG_MIN or > LONG_MAX). To capture
>>>>> both cases it looks like we need something like:
>>>>> 
>>>>> errno = 0; char* end; int probe_typess = (int) strtol(probe, &end,
>>>>> 10); if (end == probe || errno) { return JNI_ERR; }
>>>>> 
>>>>> 
>>>>> /Staffan
>>>>> 
>>>>>> -Dmitry
>>>>>> 
>>>>>> 
>>>>>> On 2014-03-25 11:31, Staffan Larsen wrote:
>>>>>>> attachListener_solaris.cpp calls atoi() and then checks errno to
>>>>>>> see if any errors occurred. The problem is that atoi() does not
>>>>>>> set errno, so some old value of errno is checked which sometimes
>>>>>>> causes the function to fail.
>>>>>>> 
>>>>>>> The fix is to replace atoi() with strtol() which does set errno.
>>>>>>> But errno is only set if an error occurred and not set to 0 in
>>>>>>> case of success. Thus, I set errno to 0 before calling strtol()
>>>>>>> and check the value afterwards.
>>>>>>> 
>>>>>>> Verified with a JPRT run.
>>>>>>> 
>>>>>>> webrev: http://cr.openjdk.java.net/~sla/8038296/webrev.00/ bug:
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8038296
>>>>>>> 
>>>>>>> Thanks, /Staffan
>>>>>>> 
>>>>>> 
>>>>>> -- Dmitry Samersoff Oracle Java development team, Saint Petersburg,
>>>>>> Russia * I would love to change the world, but they won't give me
>>>>>> the sources.
>>>> 
>>>> -- 
>>>> Dmitry Samersoff
>>>> Oracle Java development team, Saint Petersburg, Russia
>>>> * I would love to change the world, but they won't give me the sources.
>> 
> 



More information about the hotspot-runtime-dev mailing list