Review Request: RT-37788

[Danno Ferrin]

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