| Message ID | 20250325180821.1456154-4-barnabas.pocze@ideasonboard.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Hi barnabás, Thank you for the patch. On Tue, Mar 25, 2025 at 07:08:15PM +0100, Barnabás Pőcze wrote: > Instead of using a separate member variable, use `pid_ > 0` to determine > if the process is still running. Previously the value of `pid_` was not > reset to -1 when the process terminated, but since it is only meaningful > while the process is running, reset it to -1 in `Process::died()`. > > Neither `pid_` nor `running_` are exposed, so this change has no effect > on the public interface or observable behaviour. > > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > include/libcamera/internal/process.h | 1 - > src/libcamera/process.cpp | 8 +++----- > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h > index 58e3e41ce..030a1e66e 100644 > --- a/include/libcamera/internal/process.h > +++ b/include/libcamera/internal/process.h > @@ -49,7 +49,6 @@ private: > void died(int wstatus); > > pid_t pid_; > - bool running_; > enum ExitStatus exitStatus_; > int exitCode_; > > diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp > index 567b878d4..aa9e8f519 100644 > --- a/src/libcamera/process.cpp > +++ b/src/libcamera/process.cpp > @@ -208,7 +208,7 @@ const struct sigaction &ProcessManager::oldsa() const > */ > > Process::Process() > - : pid_(-1), running_(false), exitStatus_(NotExited), exitCode_(0) > + : pid_(-1), exitStatus_(NotExited), exitCode_(0) > { > } > > @@ -240,7 +240,7 @@ int Process::start(const std::string &path, > { > int ret; > > - if (running_) > + if (pid_ > 0) > return 0; > > int childPid = fork(); > @@ -252,8 +252,6 @@ int Process::start(const std::string &path, > pid_ = childPid; > ProcessManager::instance()->registerProcess(this); > > - running_ = true; > - > return 0; > } else { > if (isolate()) > @@ -327,7 +325,7 @@ int Process::isolate() > */ > void Process::died(int wstatus) > { > - running_ = false; > + pid_ = -1; > exitStatus_ = WIFEXITED(wstatus) ? NormalExit : SignalExit; > exitCode_ = exitStatus_ == NormalExit ? WEXITSTATUS(wstatus) : -1; >
diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h index 58e3e41ce..030a1e66e 100644 --- a/include/libcamera/internal/process.h +++ b/include/libcamera/internal/process.h @@ -49,7 +49,6 @@ private: void died(int wstatus); pid_t pid_; - bool running_; enum ExitStatus exitStatus_; int exitCode_; diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp index 567b878d4..aa9e8f519 100644 --- a/src/libcamera/process.cpp +++ b/src/libcamera/process.cpp @@ -208,7 +208,7 @@ const struct sigaction &ProcessManager::oldsa() const */ Process::Process() - : pid_(-1), running_(false), exitStatus_(NotExited), exitCode_(0) + : pid_(-1), exitStatus_(NotExited), exitCode_(0) { } @@ -240,7 +240,7 @@ int Process::start(const std::string &path, { int ret; - if (running_) + if (pid_ > 0) return 0; int childPid = fork(); @@ -252,8 +252,6 @@ int Process::start(const std::string &path, pid_ = childPid; ProcessManager::instance()->registerProcess(this); - running_ = true; - return 0; } else { if (isolate()) @@ -327,7 +325,7 @@ int Process::isolate() */ void Process::died(int wstatus) { - running_ = false; + pid_ = -1; exitStatus_ = WIFEXITED(wstatus) ? NormalExit : SignalExit; exitCode_ = exitStatus_ == NormalExit ? WEXITSTATUS(wstatus) : -1;
Instead of using a separate member variable, use `pid_ > 0` to determine if the process is still running. Previously the value of `pid_` was not reset to -1 when the process terminated, but since it is only meaningful while the process is running, reset it to -1 in `Process::died()`. Neither `pid_` nor `running_` are exposed, so this change has no effect on the public interface or observable behaviour. Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- include/libcamera/internal/process.h | 1 - src/libcamera/process.cpp | 8 +++----- 2 files changed, 3 insertions(+), 6 deletions(-)