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

[Staffan Larsen]

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.
>