Thanks, Chris! On 8/7/14 9:24 AM, Chris Hegarty wrote:

This looks ok to me Brent, and I agree with your conservative approach.

-Chris.

On 6 Aug 2014, at 22:45, Brent Christian <brent.christian@oracle.com> 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