| Message ID | 20250722083441.1716976-1-barnabas.pocze@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Barnabás, Thanks for the patch. Quoting Barnabás Pőcze (2025-07-22 17:34:41) > Only try to send the "Exit" message if the `IPCPipeUnixSocket` object > exists. If the constructor fails, then `ipc_` might not be initialized, Right, I missed that when I originally wrote this... > which would lead to a nullptr dereference. This can still result in an > `ipc_->sendAsync()` call even if `ipc_` is unconnected, but that only > reports an error, and is still better than the status quo. > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=276 > Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> > --- > .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > index 9a3aadbd2..ba740a0de 100644 > --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > @@ -96,7 +96,7 @@ namespace {{ns}} { > > {{proxy_name}}::~{{proxy_name}}() > { > - if (isolate_) { > + if (ipc_) { I wonder if ipc_ && isolate_ would be better? Thanks, Paul > IPCMessage::Header header = > { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ }; > IPCMessage msg(header); > -- > 2.50.1 >
Hi 2025. 07. 22. 11:33 keltezéssel, Paul Elder írta: > Hi Barnabás, > > Thanks for the patch. > > Quoting Barnabás Pőcze (2025-07-22 17:34:41) >> Only try to send the "Exit" message if the `IPCPipeUnixSocket` object >> exists. If the constructor fails, then `ipc_` might not be initialized, > > Right, I missed that when I originally wrote this... > >> which would lead to a nullptr dereference. This can still result in an >> `ipc_->sendAsync()` call even if `ipc_` is unconnected, but that only >> reports an error, and is still better than the status quo. >> >> Bug: https://bugs.libcamera.org/show_bug.cgi?id=276 >> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >> --- >> .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >> index 9a3aadbd2..ba740a0de 100644 >> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >> @@ -96,7 +96,7 @@ namespace {{ns}} { >> >> {{proxy_name}}::~{{proxy_name}}() >> { >> - if (isolate_) { >> + if (ipc_) { > > I wonder if ipc_ && isolate_ would be better? Hmm... I would go the exact opposite way, and remove `isolate_` altogether. Regards, Barnabás Pőcze > > > Thanks, > > Paul > >> IPCMessage::Header header = >> { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ }; >> IPCMessage msg(header); >> -- >> 2.50.1 >>
Hi 2025. 07. 22. 11:41 keltezéssel, Barnabás Pőcze írta: > Hi > > 2025. 07. 22. 11:33 keltezéssel, Paul Elder írta: >> Hi Barnabás, >> >> Thanks for the patch. >> >> Quoting Barnabás Pőcze (2025-07-22 17:34:41) >>> Only try to send the "Exit" message if the `IPCPipeUnixSocket` object >>> exists. If the constructor fails, then `ipc_` might not be initialized, >> >> Right, I missed that when I originally wrote this... >> >>> which would lead to a nullptr dereference. This can still result in an >>> `ipc_->sendAsync()` call even if `ipc_` is unconnected, but that only >>> reports an error, and is still better than the status quo. >>> >>> Bug: https://bugs.libcamera.org/show_bug.cgi?id=276 >>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> >>> --- >>> .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >>> index 9a3aadbd2..ba740a0de 100644 >>> --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >>> +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >>> @@ -96,7 +96,7 @@ namespace {{ns}} { >>> {{proxy_name}}::~{{proxy_name}}() >>> { >>> - if (isolate_) { >>> + if (ipc_) { >> >> I wonder if ipc_ && isolate_ would be better? > > Hmm... I would go the exact opposite way, and remove `isolate_` altogether. I have a prototype that does that, so I would like to keep it `if (ipc_)`. Please see the second version. Regards, Barnabás Pőcze > > > Regards, > Barnabás Pőcze > > >> >> >> Thanks, >> >> Paul >> >>> IPCMessage::Header header = >>> { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ }; >>> IPCMessage msg(header); >>> -- >>> 2.50.1 >>> >
diff --git a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl index 9a3aadbd2..ba740a0de 100644 --- a/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/codegen/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -96,7 +96,7 @@ namespace {{ns}} { {{proxy_name}}::~{{proxy_name}}() { - if (isolate_) { + if (ipc_) { IPCMessage::Header header = { static_cast<uint32_t>({{cmd_enum_name}}::Exit), seq_++ }; IPCMessage msg(header);
Only try to send the "Exit" message if the `IPCPipeUnixSocket` object exists. If the constructor fails, then `ipc_` might not be initialized, which would lead to a nullptr dereference. This can still result in an `ipc_->sendAsync()` call even if `ipc_` is unconnected, but that only reports an error, and is still better than the status quo. Bug: https://bugs.libcamera.org/show_bug.cgi?id=276 Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> --- .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)