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

[gary.adams at oracle.com]

More work is needed here. I have not tracked down how detach is actually 
used.
The previous path as null string was a flag to reconnect (?).

On 12/19/17 5:23 PM, Chris Plummer wrote:
> 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
>>>
>>>
>>
>
>