This looks ok to me Brent, and I agree with your conservative approach. -Chris.
On 6 Aug 2014, at 22:45, Brent Christian <[email protected]> wrote: > Please review my fix for: > > Bug: > https://bugs.openjdk.java.net/browse/JDK-8034032 > Webrev: > http://cr.openjdk.java.net/~bchristi/8034032/webrev.0/ > > > Within jdk/src/macosx/native/java/util/prefs/MacOSXPreferencesFile.m, > there is a pattern of making blocks of toCF() calls, where > later toCF() calls are made when there could be an exception pending from an > earlier toCF() call. > > toCF()creates a CFString from a Java String. If an error occurs, it will > raise an exception by calling the throwIfNull() macro: > > #define throwIfNull(var, msg) \ > do { \ > if (var == NULL) { \ > throwOutOfMemoryError(env, msg); \ > goto bad##var; \ > } \ > } while (0) > > Note: "##var" is substituted for whatever text is serving as "var", and > throwIfNull() is meant to jump to the cleanup portion of a function (e.g. > "badvar") to do cleanup before exiting. > > This goto behavior is used more as intended elsewhere in > MacOSXPreferencesFile.m. toCF() doesn't have such cleanup to do. > > An example of a series of calls to toCF(): > > 650 CFStringRef path = toCF(env, jpath); > 651 CFStringRef child = toCF(env, jchild); > 652 CFStringRef name = toCF(env, jname); > > It's not until later that we check that everything turned out OK: > > 661 if (!path || !child || !name) goto badparams; > > If one toCF() call throws an OOME exception, it will still be pending when > the next call to toCF() makes a JNI call, (GetStringLength() in this case). > > I believe the right thing to do here is to check the success of each toCF() > call before making the next toCF() call. I've left the existing null checks > / 'goto badparams' as is. > > I also clear any pending exceptions in throwOutOfMemoryError(). > > Thanks, > -Brent >
