Here is an updated webrev which incorporates Dmitry’s feedback: http://cr.openjdk.java.net/~sla/8038296/webrev.01/
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.
