linux-proc readlink patch

linux-proc readlink patch

[Daniel Jacobowitz]

This patch:

2002-01-08  Michael Snyder  <msnyder@redhat.com>

        * linux-proc.c (child_pid_to_exec_file): Use readlink to get the
        real name of the executable, rather than the /proc name.

(which I never saw posted on gdb-patches; was that an artifact of the email
breakage yesterday?)

seems to have at least one problem.

(1) It introduces a memory leak, if I understand the semantics of cleanups
correctly.  make_cleanup's cleanups will only be called (according to the
comment in utils.c) after a failed command.

(2) It is not, IIRC, always correct in the case of chroots.  Handling for
this has changed across Linux versions several times.  On 2.2 it seems to be
correct (to my surprise, actually), but I believe it is not on 2.0.  Do we
care?  Probably not, as 2.0 is now -very- old.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: linux-proc readlink patch

[Andrew Cagney]

> This patch:
> 
> 2002-01-08  Michael Snyder  <msnyder@redhat.com>
> 
> * linux-proc.c (child_pid_to_exec_file): Use readlink to get the
>         real name of the executable, rather than the /proc name.
> 
> (which I never saw posted on gdb-patches; was that an artifact of the email
> breakage yesterday?)
> 
> seems to have at least one problem.
> 
> (1) It introduces a memory leak, if I understand the semantics of cleanups
> correctly.  make_cleanup's cleanups will only be called (according to the
> comment in utils.c) after a failed command.


Since not looking at the code is current vogue .... :-)

People use cleanups to mimic two exception behavours:

	TRY  ... EXCEPT ... END
		you'll see a discard_cleanups()

	TRY ... ALWAYS ... END
		you'll see a do_cleanups ()

it was probably a forget me.


> (2) It is not, IIRC, always correct in the case of chroots.  Handling for
> this has changed across Linux versions several times.  On 2.2 it seems to be
> correct (to my surprise, actually), but I believe it is not on 2.0.  Do we
> care?  Probably not, as 2.0 is now -very- old.


I don't know if we're really that worried about mimicing old broken 
kernel behavour (at least until we get a real user complaint).

enjoy,
Andrew

Re: linux-proc readlink patch

[Michael Snyder]

Daniel Jacobowitz wrote:
> 
> This patch:
> 
> 2002-01-08  Michael Snyder  <msnyder@redhat.com>
> 
>         * linux-proc.c (child_pid_to_exec_file): Use readlink to get the
>         real name of the executable, rather than the /proc name.
> 
> (which I never saw posted on gdb-patches; was that an artifact of the email
> breakage yesterday?)

Coulda been.  I sent so many yesterday (and had to resend so many of
them)
that I can't remember.

> 
> seems to have at least one problem.
> 
> (1) It introduces a memory leak, if I understand the semantics of cleanups
> correctly.  make_cleanup's cleanups will only be called (according to the
> comment in utils.c) after a failed command.

Nope, that's not the semantics.  Cleanups are always done, no later than
when the command is finished executing (if not earlier).  I even checked
to make sure that these were done.  There's no memory leak.


> (2) It is not, IIRC, always correct in the case of chroots.  Handling for
> this has changed across Linux versions several times.  On 2.2 it seems to be
> correct (to my surprise, actually), but I believe it is not on 2.0.  Do we
> care?  Probably not, as 2.0 is now -very- old.

Well, if it fails, the code falls back to using the original /proc name.

Re: linux-proc readlink patch

[Daniel Jacobowitz]

On Wed, Jan 09, 2002 at 02:18:53PM -0800, Michael Snyder wrote:
> Nope, that's not the semantics.  Cleanups are always done, no later than
> when the command is finished executing (if not earlier).  I even checked
> to make sure that these were done.  There's no memory leak.

Well, the comments in utils.c are wrong, then :)

> > (2) It is not, IIRC, always correct in the case of chroots.  Handling for
> > this has changed across Linux versions several times.  On 2.2 it seems to be
> > correct (to my surprise, actually), but I believe it is not on 2.0.  Do we
> > care?  Probably not, as 2.0 is now -very- old.
> 
> Well, if it fails, the code falls back to using the original /proc name.

By fails, I meant that the output of readlink was not useful.  But I
don't think this is a concern.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: linux-proc readlink patch

[Kevin Buettner]

On Jan 9,  6:10pm, Daniel Jacobowitz wrote:

> On Wed, Jan 09, 2002 at 02:18:53PM -0800, Michael Snyder wrote:
> > Nope, that's not the semantics.  Cleanups are always done, no later than
> > when the command is finished executing (if not earlier).  I even checked
> > to make sure that these were done.  There's no memory leak.
> 
> Well, the comments in utils.c are wrong, then :)
> 
> > > (2) It is not, IIRC, always correct in the case of chroots.  Handling for
> > > this has changed across Linux versions several times.  On 2.2 it seems to be
> > > correct (to my surprise, actually), but I believe it is not on 2.0.  Do we
> > > care?  Probably not, as 2.0 is now -very- old.
> > 
> > Well, if it fails, the code falls back to using the original /proc name.
> 
> By fails, I meant that the output of readlink was not useful.  But I
> don't think this is a concern.

So the link was just wrong?  If so, then opening /proc/<pid>/exe won't work
either, right?

If these old kernels managed to somehow open the right file but provide
an incorrect link via readlink(), I suppose we could stat() the file
to see if it exists.

Kevin

Re: linux-proc readlink patch

[Daniel Jacobowitz]

On Wed, Jan 09, 2002 at 04:40:12PM -0700, Kevin Buettner wrote:
> On Jan 9,  6:10pm, Daniel Jacobowitz wrote:
> 
> > On Wed, Jan 09, 2002 at 02:18:53PM -0800, Michael Snyder wrote:
> > > Nope, that's not the semantics.  Cleanups are always done, no later than
> > > when the command is finished executing (if not earlier).  I even checked
> > > to make sure that these were done.  There's no memory leak.
> > 
> > Well, the comments in utils.c are wrong, then :)
> > 
> > > > (2) It is not, IIRC, always correct in the case of chroots.  Handling for
> > > > this has changed across Linux versions several times.  On 2.2 it seems to be
> > > > correct (to my surprise, actually), but I believe it is not on 2.0.  Do we
> > > > care?  Probably not, as 2.0 is now -very- old.
> > > 
> > > Well, if it fails, the code falls back to using the original /proc name.
> > 
> > By fails, I meant that the output of readlink was not useful.  But I
> > don't think this is a concern.
> 
> So the link was just wrong?  If so, then opening /proc/<pid>/exe won't work
> either, right?
> 
> If these old kernels managed to somehow open the right file but provide
> an incorrect link via readlink(), I suppose we could stat() the file
> to see if it exists.

The link was 'magic' - its text was something along the lines of a
device and inode number, I believe.  It's not worth worrying about
unless someone actually reports it broken, I think.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: linux-proc readlink patch

[Michael Elizabeth Chastain]

Daniel Jacobowitz writes:
> The link was 'magic' - its text was something along the lines of a
> device and inode number, I believe.  It's not worth worrying about
> unless someone actually reports it broken, I think.

I'm getting a nostalgia rush.

My recollection is the same as Daniel's.  /proc/$pid/exe has been a
symlink for a long time, since 1996 or so.  And it's been usable all
that time if you just open up the file and use it.

The original name was indeed "device:inode" which was unusable.  I don't
know when the transitition to a real file name took place.  The last
two stable versions of the kernel (2.2 and 2.4) have real file names
instead of "device:inode" and I feel comfortable relying on a feature
that is in the past two major versions.

Michael C

Re: linux-proc readlink patch

[Michael Snyder]

Michael Elizabeth Chastain wrote:
> 
> Daniel Jacobowitz writes:
> > The link was 'magic' - its text was something along the lines of a
> > device and inode number, I believe.  It's not worth worrying about
> > unless someone actually reports it broken, I think.
> 
> I'm getting a nostalgia rush.
> 
> My recollection is the same as Daniel's.  /proc/$pid/exe has been a
> symlink for a long time, since 1996 or so.  And it's been usable all
> that time if you just open up the file and use it.
> 
> The original name was indeed "device:inode" which was unusable.  I don't
> know when the transitition to a real file name took place.  The last
> two stable versions of the kernel (2.2 and 2.4) have real file names
> instead of "device:inode" and I feel comfortable relying on a feature
> that is in the past two major versions.

Whew!  Thank you, Mr. History Buff!  ;-)