Review Request: RT-37788

[Petr Pchelko]

Hello, Danno.

I’ve noticed that you are leaking userDefaults and expandedOptions objects. 
Adding autorelease to them would save you some memory.

With best regards. Petr.

> On Jul 3, 2014, at 1:43 AM, Danno Ferrin <danno.ferrin at oracle.com> wrote:
> 
> Yes, this has to be fixed in native code.  8u40 it is then.
> 
> I can make it cause a crash, but that starts with shooting myself in the foot, and not much can be done to fix it (by passing in bogus VM arguments).
> 
> 
> On Jul 2, 2014, at 3:40 PM, Stephen F Northover <steve.x.northover at oracle.com> wrote:
> 
>> Personally, I wouldn't change any native code at this point unless it was fixing a crash.  The review is for 8u40, correct?
>> 
>> Steve
>> 
>> On 2014-07-02, 5:38 PM, Chris Bensen wrote:
>>> I’m not sure about for 8u20. Seems fairly straight forward, and your Obj-C seems as good as any Obj-C. My only complaint at the moment is the following:
>>> 358     if ([pathParts count] > 2) {
>>> 359         // for 3 or more steps, the domain is first.second.third and the keys are "/first/second/third/", "fourth/", "fifth/"... etc
>>> 360         persistentDomain = [NSString stringWithFormat: @"%@.%@.%@", [pathParts objectAtIndex: 0],
>>> 361                             [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]];
>>> 362         
>>> 363         [dictPath replaceObjectAtIndex: 0 withObject: [NSString stringWithFormat:@"/%@/%@/%@", [pathParts objectAtIndex: 0],
>>> 364                                                        [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]]];
>>> 365         [dictPath removeObjectAtIndex: 2];
>>> 366         [dictPath removeObjectAtIndex: 1];
>>> 367     } else {
>>> 368         // for 1 or two steps, the domain is first.second.third and the keys are "/", "first/", "second/"
>>> 369         persistentDomain = @DEFAULT_JAVA_PREFS_DOMAIN;
>>> 370         [dictPath insertObject: @"" atIndex:0];
>>> 371     }
>>> 
>>> what if [pathParts count] is 0? I’d probably do a switch:
>>> 
>>> switch ([pathParts count]) {
>>>  case 0:
>>>     //error
>>>     return/break;
>>>  case 1:
>>>  case 2:
>>> 368         // for 1 or two steps, the domain is first.second.third and the keys are "/", "first/", "second/"
>>> 369         persistentDomain = @DEFAULT_JAVA_PREFS_DOMAIN;
>>> 370         [dictPath insertObject: @"" atIndex:0];
>>>  default:
>>> 359         // for 3 or more steps, the domain is first.second.third and the keys are "/first/second/third/", "fourth/", "fifth/"... etc
>>> 360         persistentDomain = [NSString stringWithFormat: @"%@.%@.%@", [pathParts objectAtIndex: 0],
>>> 361                             [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]];
>>> 362         
>>> 363         [dictPath replaceObjectAtIndex: 0 withObject: [NSString stringWithFormat:@"/%@/%@/%@", [pathParts objectAtIndex: 0],
>>> 364                                                        [pathParts objectAtIndex: 1], [pathParts objectAtIndex: 2]]];
>>> 365         [dictPath removeObjectAtIndex: 2];
>>> 366         [dictPath removeObjectAtIndex: 1];
>>> 
>>> }
>>> 
>>> 
>>> Make sense? Clear as mud?
>>> 
>>> Chris
>>> 
>>> 
>>> On Jul 2, 2014, at 2:15 PM, Danno Ferrin <danno.ferrin at oracle.com> wrote:
>>> 
>>>> Chris, Kevin, Steve,
>>>> 
>>>> Please review this fix for RT-37788.  Since I am not an objective C any comments are welcome.  Also, please consider if this is too much for an 8u20 fix (the diff is against the current 8u40 codebase).
>>>> 
>>>> Webrev: http://cr.openjdk.java.net/~shemnon/RT-37788/webrev.00/
>>>> JIRA: https://javafx-jira.kenai.com/browse/RT-37788
>>>> 
>>>> —Danno