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

[Fabian Kilger <kilger@sec.in.tum.de>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2848 bytes --]


On 6/11/25 11:58, Andrew Burgess wrote:
> Andrew Burgess <aburgess@redhat.com> writes:
> 
>> Fabian Kilger <kilger@sec.in.tum.de> writes:
>>
>>> While looking at it, I've noticed all implementations of stat functions
>>> actually use lstat and not stat. This maybe should be modified in the
>>> namespace case as well and I'll be changing this for v2. However, I
>>> could not directly find a rationale behind why *_stat functions call
>>> lstat instead. Possibly, it might make sense renaming the
>>> target_fileio_stat to target_fileio_lstat as well, though this would be
>>> an independent change.
>>
>> Without digging into the history I say for sure why it's lstat over
>> stat.
>>
>> I agree with you that renaming the API might make some sense, though I
>> suspect we're really a little stuck, the remote protocol packet is
>> called 'stat', which is unfortunate, we cannot really change that.
> 
> OK, so I dug into the history, and it would appear that (a) it was just
> a mistake, and (b) it was all my fault :-/
> 
> The code in question does want lstat, so really the whole API should
> have been called lstat.
> 
> Most of this can easily be fixed, but the remote protocol part we're
> unfortunately stuck with.  Though I wonder if we can "fix" this by just
> have a 'stat' and 'lstat' remote protocol packet, change 'stat' to do an
> actual 'stat', then on the GDB side we'll change the internal target API
> to be called 'lstat'.  This would effectively orphan the remote 'stat'
> packet, as nobody would use it, but that's OK I think, and only minimal
> overhead.  I'll take a look at rolling something like this together.
> 
> For you, right now, you should name your functions 'stat', but do an
> 'lstat' to be consistent.
> 
> Thanks,
> Andrew
> 

Following the same logic of '"correct" style takes precedent over
matching "incorrect" style', would it not make sense to start using
lstat in the name for those functions that actually perform lstat? The
code would be overall more clear if the functions are called _lstat and
perform lstat imo. In case a 'lstat' remote protocol packet will be
added, it would also be already clear if the existing functions are
already named lstat. Especially in the namespace helper code and the
communication protocol this lstat/stat name confusion would spread in a
call chain of like 5 functions.

Maybe my point would be more clear if I submit the current version of
the patch.

Best,
Fabian


-- 
Fabian Kilger, M.Sc.
Wissenschaftlicher Mitarbeiter

Technische Universität München
TUM School of Computation, Information and Technology
Chair of IT Security

Boltzmannstraße 3
Raum 01.08.053
85748 Garching (bei München)

Tel. +49 (0)89 289-18587
Fax +49 (0)89 289-18579

kilger@sec.in.tum.de
www.sec.in.tum.de

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 653 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]