Review Request: RT-37788

[Stephen F Northover]

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 
> <mailto: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/ 
>> <http://cr.openjdk.java.net/%7Eshemnon/RT-37788/webrev.00/>
>> JIRA: https://javafx-jira.kenai.com/browse/RT-37788
>>
>> —Danno
>