RFR JDK-8034032 : Check src/macosx/native/java/util/prefs/MacOSXPreferencesFile.m for JNI pending issues

[Brent Christian]

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