One more suggestion.
I like the linux variant which keeps the findSocketFile() method.
Would it better to unify things and do the same for aix and
macosx?
Thanks,
Serguei
On 12/21/17 15:37, [email protected] wrote:
A
refreshed webrev is available
Webrev: http://cr.openjdk.java.net/~gadams/8188856/webrev.02/
- added solaris version of the file which had the same
original error reporting
the wrong socket file name
- restored the linux detach/execute logic
- updated the macosx and aix detach/execute logic
- a few minor differences between the various platform
specific files
curly braces and exception args.
Ran into some issues with mach5 testing which will require
another
test run tomorrow.
On 12/19/17, 10:00 AM, Gary Adams wrote:
A refreshed webrev is available
Webrev: http://cr.openjdk.java.net/~gadams/8188856/webrev.01/
On 12/18/17, 4:38 PM, Chris Plummer wrote:
On 12/18/17 1:22 PM, [email protected]
wrote:
On 12/18/17 2:26 PM, Chris Plummer
wrote:
Hi Gary,
On 12/18/17 6:47 AM, Gary Adams wrote:
Here's a simple fix to correct
the error message when the java_pid socket
is not found. The code previously reported the
attach_pid file name
rather than the socket file name that was not found.
Issue: https://bugs.openjdk.java.net/browse/JDK-8188856
Webrev: http://cr.openjdk.java.net/~gadams/8188856/webrev.00/
I don't understand why you couldn't simply have changed
the f.getPath() reference to "path". From what I can
see, there is no difference between "path" and
"socket_name". The problem you are fixing is that the
error message prints f.getPath(), but that refers to the
attach file and the error message should refer to the
socket file. You've correct this, but have done so in a
round about way. Above the error message (in two places)
exists:
path = findSocketFile(pid, ns_pid);
So "path" is what you want. You have indirectly fixed
the problem by having findSocketFile() store the path in
socket_name, and then you print socket_name, but why not
just do the direct fix and print "path".
Also, the copyrights need to be updated.
thanks,
Chris
The problem was path is used to hold the constructed file
name
if it is confirmed to exist in the file system. Otherwise
it is passed back from
findSocketFile as a null to indicate the socket file was
not found.
I could refactor where the existence check is handled, but
it seemed like the least
invasive change to simply save the socket name for the
printing in the error case.
Ah, right. Obviously "path" is null when you want to print
the error message. I guess I have a hard time with "path"
and "socket_name" for the most part being the same (except
"path" can end up being null), yet they have two completely
dissimilar names. Why not rename path to socket_path and
then add saved_socket_path, or something like that. No
changes to your current logic, just to the names being used.
Or, have findSocketFile() actually return the File, and then
rename path to socket_file and also rename socket_name to
socket_path. The truth of the matter is the result of
findSocketFile() is only used to see if the socket file
exists. You could even change it to return a boolean.
Chris
|
