RFR: JDK-8188856: Incorrect file path in an exception message when .java_pid is not accessible on Unix

[Chris Plummer]

Hi Gary,

I'm not sure about the detach() and execute() changes on linux (how can 
this.path references just be stripped and the code still work properly), 
and how does this code continue to work on AIX and MacOSX when "path" 
has been removed but is still referenced. Shouldn't this.path just be 
replaced with this.socket_name?

thanks,

Chris

On 12/19/17 7: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, gary.adams at oracle.com 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
>>
>>
>