RFR - Changes to address 8024007, minor fixes for static agent code

[BILL PITTORE]

Dan had a few comments after I pushed the original code for static agent 
support, bugid 8014135. These changes address those comments. Webrev at:

http://cr.openjdk.java.net/~bpittore/8024007/webrev.00/

thanks,
  bill

On 8/29/2013 12:55 PM, Daniel D. Daugherty wrote:
> On 8/21/13 1:33 PM, BILL PITTORE wrote:
>> http://cr.openjdk.java.net/~bpittore/8014135/hotspot/webrev.04/
>
> Sorry for the very late review.
>
> Nothing critical or "must do" here.
>
> src/share/vm/prims/jvmti.xml
>     line 460: The VM will invoke the Agent_OnUnload_L function of the 
> agent, if such
>     line 461: a function is exported, at the same point during startup 
> as it would
>     line 462: have called the dynamic entry point Agent_OnUnLoad.
>
>         The "at the same point during startup" part caught my eye. Not
>         sure why you're constraining this to "during startup". Perhaps
>         "during VM execution" instead.
>
>         I'm having a little bit of difficulty groking the idea of calling
>         an unload function when the statically linked library can really
>         be unloaded. However, I can see the need for a hook of some sort
>         that can be called when the library would be logically unloaded.
>
> src/share/vm/runtime/arguments.hpp
>     line 147: void set_static_lib(bool static_lib)      { 
> _is_static_lib = static_lib; }
>
>         The usual HotSpot style would be:
>
>           void set_static_lib(bool is_static_lib)   { _is_static_lib = 
> is_static_lib; }
>
>         i.e., same name differentiated by a leading '_' only.
>
> src/share/vm/prims/jvmtiExport.cpp
>     No comments.
>
> src/share/vm/runtime/thread.cpp
>     line 3707: // First check to see if agent is statcally linked into 
> executable
>         typo: 'statcally' -> 'statically'
>
> src/share/vm/runtime/os.hpp
>     No comments.
>
> src/share/vm/runtime/os.cpp
>     line 455: void* os::find_agent_function(AgentLibrary *agent_lib, 
> bool check_lib,
>     line 456:                               const char *syms[], size_t 
> syms_len) {
>         assert(agent_lib != NULL, "sanity check");
>
>         would be a good addition.
>
>     line 481: bool os::find_builtin_agent(AgentLibrary *agent_lib, 
> const char *syms[],
>     line 482:                             size_t syms_len) {
>         assert(agent_lib != NULL, "sanity check");
>
>         would be a good addition.
>
>     This block:
>
>     495   ret = find_agent_function(agent_lib, true, syms, syms_len);
>     496   agent_lib->set_os_lib(save_handle);
>     497   if (ret != NULL) {
>     498     // Found an entry point like Agent_OnLoad_lib_name so we 
> have a static agent
>     499     agent_lib->set_os_lib(proc_handle);
>     500     agent_lib->set_valid();
>     501     agent_lib->set_static_lib(true);
>     502     return true;
>     503   }
>     504   return false;
>
>     would be more clear as:
>
>     495   ret = find_agent_function(agent_lib, true, syms, syms_len);
>     496   if (ret != NULL) {
>     497     // Found an entry point like Agent_OnLoad_lib_name so we 
> have a static agent
>     498     agent_lib->set_valid();
>     499     agent_lib->set_static_lib(true);
>     500     return true;
>     501   }
>     502   agent_lib->set_os_lib(save_handle);
>     503   return false;
>
>
> src/os/posix/vm/os_posix.cpp
>     No comments.
>
> src/os/windows/vm/os_windows.cpp
>     line 5427: // Need to check for C:
>         Need to check for drive prefix. (not just 'c:')
>