Code Review fix for 8005044 remove crufty '_g' support from HS runtime code

[harold seigel]

Hi Ron,

I'm ok with using snprinft().

Another nit: Is it worth checking that all of "/hotspot/libjvm.so" could 
get copied to buf?  (i.e.:  that buflen-len > strlen("/hotspot/libjvm.so")

Thanks, Harold

On 12/19/2012 9:17 AM, Ron Durbin wrote:
>
> Harold,
>
> Did you have any more points or counter points to make on the use of 
> snprintf and strncpy ?
>
> Ron
>
> *From:*Ron Durbin
> *Sent:* Tuesday, December 18, 2012 4:50 PM
> *To:* Daniel Daugherty; hotspot-runtime-dev at openjdk.java.net; 
> build-dev; serviceability-dev at openjdk.java.net
> *Subject:* RE: Code Review fix for 8005044 remove crufty '_g' support 
> from HS runtime code
>
> Thanks for the quick reviews by all.
>
> I will agree with Dan on this one, sprint will stay.
>
> Ron
>
> *From:*Daniel D. Daugherty
> *Sent:* Tuesday, December 18, 2012 4:03 PM
> *To:* hotspot-runtime-dev at openjdk.java.net 
> <mailto:hotspot-runtime-dev at openjdk.java.net>; build-dev; 
> serviceability-dev at openjdk.java.net 
> <mailto:serviceability-dev at openjdk.java.net>
> *Subject:* Re: Code Review fix for 8005044 remove crufty '_g' support 
> from HS runtime code
>
> Sorry, I lost my manners somewhere... :-(
>
> Thanks for the fast review!
>
> Dan
>
> On 12/18/12 3:57 PM, Daniel D. Daugherty wrote:
>
>     Adding back in the other aliases...
>
>     Harold,
>
>     The equivalent of:
>
>         snprintf(buf, n, str);
>
>     is:
>
>         strncpy(buf, str, n - 1);
>         buf[n - 1] = '\0';
>
>     because snprintf() will only write out (n - 1) bytes from
>     'str' to 'buf' and then write a NUL in the last slot.
>     strncpy() copies up to 'n' bytes from 'str' to 'buf'.
>     strncpy() will not copy past a NUL character in 'str', but
>     it will also not NUL terminate 'buf' if a NUL is not found
>     in 'str' before the limit 'n' is reached.
>
>     Dan
>
>     On 12/18/12 2:48 PM, harold seigel wrote:
>
>         Hi Ron,
>
>         I think these calls to snprintf() in os_solaris.cpp and the
>         other os_....cpp files could be replaced with calls to
>         strncpy().  It might be a little simpler.
>
>         Otherwise, it looks good.
>
>         Thanks, Harold
>
>         2539 if (0 == access(buf, F_OK)) {
>         2540 // Use current module name "libjvm.so"
>         2541 len = strlen(buf);
>         2542 *snprintf(buf + len, buflen-len, "/hotspot/libjvm.so");*
>         2543 } else {
>         2544 // Go back to path of .so
>         2545 realpath((char *)dlinfo.dli_fname, buf); 2546 }
>
>         On 12/18/2012 3:46 PM, Daniel D. Daugherty wrote:
>
>         Greetings,
>
>         I'm sponsoring this code review request from Ron Durbin. This
>         change
>         is targeted at JDK8/HSX-25 in the RT_Baseline repo.
>
>         Dan
>
>         Sending again with correct subject line, bug URLs and webrev URL.
>
>
>         Intro:
>
>         This set of changes removes the runtime support for generation
>         of debug versions that follow _g semantics.
>
>         Defect:
>         JDK-8005044 remove crufty '_g' support from HS runtime code
>         http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8005044
>         https://jbs.oracle.com/bugs/browse/JDK-8005044
>
>
>         Webrev
>         http://cr.openjdk.java.net/~dcubed/for_rdurbin/8005044-webrev/0 <http://cr.openjdk.java.net/%7Edcubed/for_rdurbin/8005044-webrev/0>
>
>
>
>         Details:
>         Files have been modified to remove all reference and support
>         for  debug versions that follow _g semantics.
>
>         Testing:
>
>         Passed JPRT last night:
>
>         Additional Testing In process: (suggested by Dan):
>
>         src/share/vm/runtime/arguments.cpp
>             - test with shared archive creation and use; see the e-mail
>               from Coleen
>
>         src/share/tools/ProjectCreator/ProjectCreator.java
>             - just a usage message; visual inspection of the code
>
>         src/os/windows/vm/os_windows.cpp
>             - comments only; no testing needed
>
>         src/os/{bsd,linux,solaris}/vm/os_{bsd,linux,solaris}.cpp
>             - the only code changes come into play when the "gamma"
>               launcher is used
>             - and when JAVA_HOME refers to a valid JDK, the function
>               fakes up a JVM path so that callers using the JVM path
>               to find other things in the JDK will work.
>             - I can't find any way that the actual JVM path value
>               that is returned is exposed
>             - I don't see a way to test this other than have a debug
>               printf() or manual code inspection.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/attachments/20121219/b398cb2c/attachment-0001.html