RFR: 8232846: ProcessHandle.Info command with non-English shows question marks

[Toshio 5 Nakamura]

Hi Roger, Suenaga-san,

Thank you for your comments and discussion for the issue.
I've updated webrev. Could you review it again?
tier1 tests passed.

webrev.01: http://cr.openjdk.java.net/~tnakamura/8232846/webrev.01

Best regards,
Toshio Nakamura

Yasumasa Suenaga <suenaga at oss.nttdata.com> wrote on 2020/03/27 09:10:15:

> On 2020/03/27 9:01, Roger Riggs wrote:
> > Hi,
> >
> > Please don't throw an exception.
> > It would abort being able to provide any information.
> > And who would expect or handle such an exception?
> >
> > Degrading the info or omitting it provides better service overall.
> > There is no point to throwing an exception.
>
> hProcess in QueryFullProcessImageNameW needs
> PROCESS_QUERY_INFORMATION or PROCESS_QUERY_LIMITED_INFORMATION
> access right, so I thought it is reasonable to throw runtime
> exception if the call failed.
>
> Anyway, I agree with you if throwing exception is not comfortable in
> this case for Java API.
>
>
> Thanks,
>
> Yasumasa
>
>
> > Regards, Roger
> >
> >
> > On 3/26/20 7:55 PM, Yasumasa Suenaga wrote:
> >> On 2020/03/27 0:35, Roger Riggs wrote:
> >>> Hi,
> >>>
> >>> Reading further down the reference to the section:
> >>> "Enable Long Paths in Windows 10, Version 1607, and Later"
> >>> might suggest the code be more resilient to long paths.
> >>>
> >>> The stack allocated buffer (1024) is fine, but I'd suggest
> adding code to retry in the case
> >>> of INSUFFICIENT BUFFER with a malloc'd buffer to make it more
> robust since the enabling
> >>> of long paths is environment and application specific.
> >>
> >> In addition, it's better to throw an exception if the call failed
> due to other error code.
> >>
> >> Thanks,
> >>
> >> Yasumasa
> >>
> >>
> >>> Thanks, Roger
> >>>
> >>>
> >>>
> >>> On 3/26/20 10:40 AM, Toshio 5 Nakamura wrote:
> >>>> Hi Thomas, Suenaga-san, Roger,
> >>>>
> >>>> Thank you for reviewing and comments.
> >>>>
> >>>> I checked behaviors by a small program and debugger.
> >>>> If QueryFullProcessImageNameW failed by not enough buffer,
> >>>> the API didn't change the buffer.
> >>>> It just set ERROR_INSUFFICIENT_BUFFER(0x7a) to LastError.
> >>>>
> >>>> The buffer size becomes 1024 characters (2048 bytes) by this patch.
> >>>> Should it use bigger size with malloc? 32,767 characters can belimit
[2].
> >>>> I feel 1024 is reasonable value for command's location.
> >>>>
> >>>> [2]
> >>>>
https://urldefense.com/v3/__https://docs.microsoft.com/en-us/windows/win32/fileio/naming-a-file*maximum-path-length-limitation__;Iw!!GqivPVa7Brio!PgzCcUQOjCNpcVeetU_drS4DFFVLaj0ceJBvipX7iDc_RlPtcEkdH8AzelGtzqXS
$
> >>>> Best regards,
> >>>> Toshio Nakamura
> >>>>
> >>>>> Hi,
> >>>>>
> >>>>> If the call fails, the command field in the info will not be set
(and
> >>>>> therefore null).
> >>>>>
> >>>>> I agree that a length of 512 (characters) is probably too small.
> >>>>>
> >>>>> Roger
> >>>>>
> >>>>>
> >>>>> On 3/26/20 6:37 AM, Yasumasa Suenaga wrote:
> >>>>>> Hi Nakamura-san,
> >>>>>>
> >>>>>> Your change almost looks good, but I have one question.
> >>>>>> Length of exeName (1024) is enough?
> >>>>>>
> >>>>>> According to Microsoft Doc [1], exeName might not be
null-terminated
> >>>>>> if it failed.
> >>>>>> I concern buffer overrun might occur when
QueryFullProcessImageNameW
> >>>>>> failed.
> >>>>>>
> >>>>>>
> >>>>>> Thanks,
> >>>>>>
> >>>>>> Yasumasa
> >>>>>>
> >>>>>>
> >>>>>> [1]
> >>>>
https://urldefense.proofpoint.com/v2/url?u=https-3A__docs.microsoft.com_en-2Dus_windows_win32_api_winbase_nf-2Dwinbase-2Dqueryfullprocessimagenamew&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=noFyLWDG1dMjO5TlaqM_zvW_f-8fvveS2EoZylT5DMQ&s=oc_KTGmJUjsfnLf3tjWr9DO1dwWp1TqIZgd2EiHVW14&e=

> >>>>
> >>>>>>
> >>>>>> On 2020/03/26 17:58, Toshio 5 Nakamura wrote:
> >>>>>>> Hi All,
> >>>>>>>
> >>>>>>> Could you review this change? Additionally, I'd like to ask a
sponsor
> >>>>>>> for
> >>>>>>> it, since I'm not a committer.
> >>>>>>>
> >>>>>>> Bug:
> >>>>
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugs.openjdk.java.net_browse_JDK-2D8232846&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=noFyLWDG1dMjO5TlaqM_zvW_f-8fvveS2EoZylT5DMQ&s=pn_uS0DojBk6-R6R5QwtYFJvJpaabhia-eiOtIrJO9Q&e=

> >>>>
> >>>>>>> Webrev:
> >>>>
https://urldefense.proofpoint.com/v2/url?u=http-3A__cr.openjdk.java.net_-7Etnakamura_8232846_webrev.00&d=DwICaQ&c=jf_iaSHvJObTbx-siA1ZOg&r=EVbFABcgo-X99_TGI2-qsMtyulHUruf8lAzMlVpVRqw&m=noFyLWDG1dMjO5TlaqM_zvW_f-8fvveS2EoZylT5DMQ&s=NylYK0Q3nFFZRVIaeUna6iaClaJm1CWJ2Zkwy-ysvv4&e=

> >>>>
> >>>>>>> Under Windows environments, ProcessHandle.Info.command shows
question
> >>>>>>> marks
> >>>>>>> if the command has non-English characters. I'd like to change the
> >>>>>>> underlying API to Unicode version.
> >>>>>>> Tier1 tests passed.
> >>>>>>>
> >>>>>>> Best regards,
> >>>>>>> Toshio Nakamura
> >>>>>>>
> >>>
> >
>