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

[Chris Plummer]

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