[7u4] Review request for CR 7134730

Greg Brown greg.x.brown at oracle.com
Fri Jan 27 09:48:47 PST 2012


> I'd actually recommend putting the Java resources (.jars and .jnilibs) into Contents/Java now, and not Contents/Resources/Java, because of issues with code-signing (Resources is designed to be a location for localized content or images, which may be changeable without breaking the bundle's code signature).

Will do.

> Also, I'd simply suggest initializing:
> + NSMutableArray *arguments = [NSMutableArray array];
> and not worry about pre-initializing the capacity, or re-assigning the variable later on. It just makes the code a lot cleaner (with no measurable performance cost, since this straight-line code is only executed once).

OK.

> I'm also a little confused by:
> +        expanded = [expanded stringByReplacingOccurrencesOfString:@" " withString:@"\\ "];
> 
> Does this mean that HotSpot needs spaces (and possibly other shell-interpreted characters) pre-escaped with backslashes for it? I really don't think this is actually the case, because our previous code did nothing like this. Please remember that the output of this tool is injected directly to HotSpot via API, and is not used to invoke a shell or a shell tool to like "java" or "javac".

That's a good point. I had added this code because I wasn't able to launch a bundle whose name contained a space - however, it didn't actually fix the problem and should probably be removed. I'll take it out.

Thanks for the review!

Greg



More information about the macosx-port-dev mailing list