RFR: JDK-8210836 : Build fails with warn_unused_result in openjdk/src/jdk.hotspot.agent/linux/native/libsaproc/ps_core.c

[JC Beyler]

I agree, thanks Jini for the clarification. And to talk about the other
case of pread in the file for completeness:

- The pread that tests <= 0 actually is correct: as you state it is in a
loop so there are two cases:
     - pread returns -1 and it is a failure, we break and after the loop we
check if there is any residual bytes that should be read, if so we fail
     - pread returns 0 because it is the end was found (the len provided is
a min of what is remaining and the map_info size)

So its test <=0 is the right thing to do.

- In the case of the code you changed, you are right; looking at the ELF
format, the section header table is at the end of the file, so you can't
have a segment that is at the end; hence it can never have a return 0 (or
it would be an error as you stated).

Thanks for taking the time to look and explain it to me :),

Looks good to me (not a reviewer though),
Jc


On Tue, Sep 18, 2018 at 10:26 AM Jini George <jini.george at oracle.com> wrote:

> Hi JC,
>
> Thank you for looking into this. Since we are reading the PT_INTERP
> segment in this case to get the path of the dynamic linker, and since
> p_filesz denotes the size of the segment (in this case, it would be the
> size of the string denoting the path of the dynamic linker), if we end
> up reading anything less than p_filesz, it should be an error. If
> pread() returns a zero denoting an EOF while we are reading in the
> PT_INTERP segment, I guess it would mean that we are dealing with a
> truncated or a possibly corrupted file.
>
> Thanks!
> Jini.
>
>
> On 9/18/2018 7:46 PM, JC Beyler 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.stuefe at gmail.com
> > <mailto:thomas.stuefe at gmail.com>> wrote:
> >
> >     Looks good. Thanks for fixing.
> >
> >     ..Thomas
> >
> >     On Tue, Sep 18, 2018 at 1:52 PM, Jini George <jini.george at oracle.com
> >     <mailto:jini.george at 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/
> >     <http://cr.openjdk.java.net/%7Ejgeorge/8210836/webrev.00/>
> >      >
> >      > A quick review would be appreciated.
> >      >
> >      > Thank you!
> >      > Jini.
> >
> >
> >
> > --
> >
> > Thanks,
> > Jc
>


-- 

Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180918/92dbd152/attachment.html>