[RFC PATCH v3 7/9] libcamera: process: Move `closeAllFdsExcept()`

[Kieran Bingham]

Quoting Barnabás Pőcze (2025-03-25 18:08:19)
> Remove `closeAllFdsExcept()` from the `Process` class and have
> it as a local function since it is no needed "outside" and does
> not depend on any part of the `Process` class.

Any impact on https://patchwork.libcamera.org/patch/22403/ ? Could you
review that please?

> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze at ideasonboard.com>
> ---
>  include/libcamera/internal/process.h |  1 -
>  src/libcamera/process.cpp            | 56 ++++++++++++++--------------
>  2 files changed, 28 insertions(+), 29 deletions(-)
> 
> diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h
> index e4f5bb979..14391d60a 100644
> --- a/include/libcamera/internal/process.h
> +++ b/include/libcamera/internal/process.h
> @@ -44,7 +44,6 @@ public:
>  private:
>         LIBCAMERA_DISABLE_COPY_AND_MOVE(Process)
>  
> -       void closeAllFdsExcept(Span<const int> fds);

It's already private though isn't it ? 

Is it just better to keep functions isolated if they aren't directly
using the class ?

>         int isolate();
>         void died(int wstatus);
>  
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 5603dc903..01503e485 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -63,6 +63,34 @@ void sigact(int signal, siginfo_t *info, void *ucontext)
>         }
>  }
>  
> +void closeAllFdsExcept(Span<const int> fds)

static? or is this already in an annoymous namespace or such ?

> +{
> +       std::vector<int> v(fds.begin(), fds.end());
> +       std::sort(v.begin(), v.end());
> +
> +       ASSERT(v.empty() || v.front() >= 0);
> +
> +       DIR *dir = opendir("/proc/self/fd");
> +       if (!dir)
> +               return;
> +
> +       int dfd = dirfd(dir);
> +
> +       struct dirent *ent;
> +       while ((ent = readdir(dir)) != nullptr) {
> +               char *endp;
> +               int fd = strtoul(ent->d_name, &endp, 10);
> +               if (*endp)
> +                       continue;
> +
> +               if (fd >= 0 && fd != dfd &&
> +                   !std::binary_search(v.begin(), v.end(), fd))
> +                       close(fd);
> +       }
> +
> +       closedir(dir);
> +}
> +
>  } /* namespace */
>  
>  void ProcessManager::sighandler()
> @@ -282,34 +310,6 @@ int Process::start(const std::string &path,
>         }
>  }
>  
> -void Process::closeAllFdsExcept(Span<const int> fds)
> -{
> -       std::vector<int> v(fds.begin(), fds.end());
> -       sort(v.begin(), v.end());
> -
> -       ASSERT(v.empty() || v.front() >= 0);
> -
> -       DIR *dir = opendir("/proc/self/fd");
> -       if (!dir)
> -               return;
> -
> -       int dfd = dirfd(dir);
> -
> -       struct dirent *ent;
> -       while ((ent = readdir(dir)) != nullptr) {
> -               char *endp;
> -               int fd = strtoul(ent->d_name, &endp, 10);
> -               if (*endp)
> -                       continue;
> -
> -               if (fd >= 0 && fd != dfd &&
> -                   !std::binary_search(v.begin(), v.end(), fd))
> -                       close(fd);
> -       }
> -
> -       closedir(dir);
> -}
> -
>  int Process::isolate()
>  {
>         int ret = unshare(CLONE_NEWUSER | CLONE_NEWNET);
> -- 
> 2.49.0
>