Re: [PATCH 2/4] vircommand: Isolate FD dir parsing into a separate function
On Fri, Sep 13, 2024 at 09:37:30AM +0200, Michal Prívozník wrote:
On 9/12/24 15:19, Martin Kletzander wrote:
> On Wed, Aug 28, 2024 at 02:16:15PM +0200, Michal Privoznik wrote:
>> So far, virCommandMassCloseGetFDsLinux() opens "/proc/self/fd",
>> iterates over it marking opened FDs in @fds bitmap. Well, we can
>> do the same on other systems (with altered path), like MacOS or
>> FreeBSD. Therefore, isolate dir iteration into a separate
>> function that accepts dir path as an argument.
>>
>> Unfortunately, this function might be unused on some systems
>> (e.g. mingw), therefore mark it as such.
>>
>
> Not in this patch. This patch should leave it in without marking it as
> unused. With that
Why not? A new function is introduced, outside of any #ifdef block, but
it's used solely in #ifdef __linux__ (for now). Or am I missing something?
Yes, it is used only if __linux__ so it should be marked as such. Later
patch should add the other systems.
The issue I have with marking it as unused is that it is then there
forever and we won't notice it being completely unused at some point.
And it does not feel too clean.
>
> Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
Michal
Attachments:
- signature.asc (application/pgp-signature — 833 bytes)