Re: [PATCH 1/2][PR GDB/32956] gdb: implement linux namespace support for fileio_stat

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

>>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:
>
> Andrew> OK, so I dug into the history, and it would appear that (a) it was just
> Andrew> a mistake, and (b) it was all my fault :-/
>
> A classic programming story, I can't tell you how many times I've been
> there.
>
> Andrew> Most of this can easily be fixed, but the remote protocol part we're
> Andrew> unfortunately stuck with.  Though I wonder if we can "fix" this by just
> Andrew> have a 'stat' and 'lstat' remote protocol packet, change 'stat' to do an
> Andrew> actual 'stat', then on the GDB side we'll change the internal target API
> Andrew> to be called 'lstat'.  This would effectively orphan the remote 'stat'
> Andrew> packet, as nobody would use it, but that's OK I think, and only minimal
> Andrew> overhead.  I'll take a look at rolling something like this together.
>
> Wouldn't this negatively affect any existing remotes that implement stat-as-lstat?
> (I don't know if any of these actually exist.)

Yes, you are correct.  But:

  - The vFile:stat documentation is pretty vague, it just says: get
    information about a file.  So if there are other remotes that
    implement this then they probably implemented 'stat' anyway as that
    would be the logical first choice.

    I would propose, as part of this change, to better document what is
    expected from 'stat' and 'lstat'.

    It could (maybe) be argued that the current 'lstat' implementation
    was a bug, and it should have been 'stat'.

  - The vFile:stat packet was only added in GDB 16, and there's only the
    one use of it from GDB.  In most cases implementing vFile:stat as an
    actual 'stat' will go unnoticed, though the GDB testsuite does rely
    on it being an actual 'lstat', so if someone is testing a custom
    remote against the GDB testsuite they might have figured out that
    they needed lstat instead of stat.

    My hope would be that adding a new vFile:lstat packet, and noting in
    the NEWS file that the vFile:stat should _really_ be 'stat' would
    allow any other remotes to course correct.

If you feel strongly though then we can for sure just leave the
vFile:stat packet implemented as 'lstat' and document it as such.

Thanks,
Andrew


>
> It might be better to just document it.
>
> Tom