I think the abbreviated form - requiring the exact number of bytes
read - is fine in this case. If you want to do it really correct,
you'd have to read in a loop (since you may read fewer bytes due to IO
stalls) and handle EINTR. But the code before certainly did not care,
so the fix does not make it worse. Since this is an urgent buildfix
I'd be fine with it in this form.

I agree, pread <= 0 is questionable too. Since the following code is
not prepared to deal with anything but a full read, comparing to
buffer size, like in the fixed example, would be better.

Cheers, Thomas


On Tue, Sep 18, 2018 at 4:16 PM, JC Beyler <jcbey...@google.com> wrote:
> Hi Jini,
>
> I was looking at the man page and am curious: it says that the method
> returns 0 if the end-of-file is reached, does that never happen in this
> case?
>
> (seems like -1 is the error code and another call to pread in the file just
> checks for <= 0; which also is weird since 0 just means end of file).
>
> Thanks,
> Jc
>
> On Tue, Sep 18, 2018 at 6:06 AM Thomas Stüfe <thomas.stu...@gmail.com>
> wrote:
>>
>> Looks good. Thanks for fixing.
>>
>> ..Thomas
>>
>> On Tue, Sep 18, 2018 at 1:52 PM, Jini George <jini.geo...@oracle.com>
>> wrote:
>> > Hi all,
>> >
>> > Please review the small change for fixing the build failure in
>> > src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c with
>> > -Werror=unused-result.
>> >
>> > https://bugs.openjdk.java.net/browse/JDK-8210836
>> > Webrev: http://cr.openjdk.java.net/~jgeorge/8210836/webrev.00/
>> >
>> > A quick review would be appreciated.
>> >
>> > Thank you!
>> > Jini.
>
>
>
> --
>
> Thanks,
> Jc

Reply via email to