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

[Jini George]

Thank you, JC and Thomas! I have pushed the change.

- Jini.


On 9/18/2018 10:59 PM, JC Beyler wrote:
> 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 
> <mailto: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>
>      > <mailto: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>
>      >     <mailto: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/>
>      >     <http://cr.openjdk.java.net/%7Ejgeorge/8210836/webrev.00/>
>      >      >
>      >      > A quick review would be appreciated.
>      >      >
>      >      > Thank you!
>      >      > Jini.
>      >
>      >
>      >
>      > --
>      >
>      > Thanks,
>      > Jc
> 
> 
> 
> -- 
> 
> Thanks,
> Jc