Message ID | 20250325180821.1456154-6-barnabas.pocze@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
Quoting Barnabás Pőcze (2025-03-25 18:08:17) > Return `-EINVAL` from `Process::start()` if any of the file > descriptors are negative as those most likely signal some > kind of issue such as missed error checking. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/libcamera/process.cpp | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp > index 716523acc..62e63255b 100644 > --- a/src/libcamera/process.cpp > +++ b/src/libcamera/process.cpp > @@ -243,6 +243,11 @@ int Process::start(const std::string &path, > if (pid_ > 0) > return -EBUSY; > > + for (int fd : fds) { > + if (fd < 0) > + return -EINVAL; > + } > + > int childPid = fork(); > if (childPid == -1) { > ret = -errno; > @@ -282,6 +287,8 @@ void Process::closeAllFdsExcept(const std::vector<int> &fds) > std::vector<int> v(fds); > sort(v.begin(), v.end()); > > + ASSERT(v.empty() || v.front() >= 0); > + > DIR *dir = opendir("/proc/self/fd"); > if (!dir) > return; > -- > 2.49.0 >
Hi Barnabás, Thank you for the patch. On Tue, Mar 25, 2025 at 07:08:17PM +0100, Barnabás Pőcze wrote: > Return `-EINVAL` from `Process::start()` if any of the file > descriptors are negative as those most likely signal some > kind of issue such as missed error checking. Feel free to go up to the 72 columns limit :-) > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/libcamera/process.cpp | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp > index 716523acc..62e63255b 100644 > --- a/src/libcamera/process.cpp > +++ b/src/libcamera/process.cpp > @@ -243,6 +243,11 @@ int Process::start(const std::string &path, > if (pid_ > 0) > return -EBUSY; > > + for (int fd : fds) { > + if (fd < 0) > + return -EINVAL; > + } > + > int childPid = fork(); > if (childPid == -1) { > ret = -errno; > @@ -282,6 +287,8 @@ void Process::closeAllFdsExcept(const std::vector<int> &fds) > std::vector<int> v(fds); > sort(v.begin(), v.end()); > > + ASSERT(v.empty() || v.front() >= 0); > + > DIR *dir = opendir("/proc/self/fd"); > if (!dir) > return;
diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 716523acc..62e63255b 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -243,6 +243,11 @@ int Process::start(const std::string &path, if (pid_ > 0) return -EBUSY; + for (int fd : fds) { + if (fd < 0) + return -EINVAL; + } + int childPid = fork(); if (childPid == -1) { ret = -errno; @@ -282,6 +287,8 @@ void Process::closeAllFdsExcept(const std::vector<int> &fds) std::vector<int> v(fds); sort(v.begin(), v.end()); + ASSERT(v.empty() || v.front() >= 0); + DIR *dir = opendir("/proc/self/fd"); if (!dir) return;
Return `-EINVAL` from `Process::start()` if any of the file descriptors are negative as those most likely signal some kind of issue such as missed error checking. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- src/libcamera/process.cpp | 7 +++++++ 1 file changed, 7 insertions(+)