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.geo...@oracle.com <mailto:jini.geo...@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.stu...@gmail.com <mailto:thomas.stu...@gmail.com>
     > <mailto:thomas.stu...@gmail.com
    <mailto: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 <mailto:jini.geo...@oracle.com>
     >     <mailto:jini.geo...@oracle.com
    <mailto: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/
    <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

Reply via email to