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 <[email protected]> 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
<[email protected]> 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.
