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

[Gary Adams]

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
>
>