| Message ID | 20240409104044.2107208-1-paul.elder@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 9dc601cf7a689fe6eef23189edf0c8a9c38dcfe1 |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Tue, Apr 09, 2024 at 07:40:44PM +0900, Paul Elder wrote: > If an event function is defined with no parameters, there would be a > compilation error complaining about unused parameters in the generated > code for the data and dataSize parameters that would normally correspond > to serialized data. Fix this by simply marking the parameters as > maybe_unused. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > index c37c4941..238cf4a5 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > @@ -235,8 +235,8 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) > } > > void {{proxy_name}}::{{method.mojom_name}}IPC( > - std::vector<uint8_t>::const_iterator data, > - size_t dataSize, > + [[maybe_unused]] std::vector<uint8_t>::const_iterator data, > + [[maybe_unused]] size_t dataSize, Do you think it would be useful to set [[maybe_unused]] only when there are no parameters, or would that be overkill ? I suppose it's called "maybe_unused" and ont "unused" for a reason, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > [[maybe_unused]] const std::vector<SharedFD> &fds) > { > {%- for param in method.parameters %}
On Tue, Apr 09, 2024 at 06:24:28PM +0300, Laurent Pinchart wrote: > Hi Paul, > > Thank you for the patch. > > On Tue, Apr 09, 2024 at 07:40:44PM +0900, Paul Elder wrote: > > If an event function is defined with no parameters, there would be a > > compilation error complaining about unused parameters in the generated > > code for the data and dataSize parameters that would normally correspond > > to serialized data. Fix this by simply marking the parameters as > > maybe_unused. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > > index c37c4941..238cf4a5 100644 > > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > > @@ -235,8 +235,8 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) > > } > > > > void {{proxy_name}}::{{method.mojom_name}}IPC( > > - std::vector<uint8_t>::const_iterator data, > > - size_t dataSize, > > + [[maybe_unused]] std::vector<uint8_t>::const_iterator data, > > + [[maybe_unused]] size_t dataSize, > > Do you think it would be useful to set [[maybe_unused]] only when there > are no parameters, or would that be overkill ? I suppose it's called > "maybe_unused" and ont "unused" for a reason, so I think it's overkill :) > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Thanks, Paul > > > [[maybe_unused]] const std::vector<SharedFD> &fds) > > { > > {%- for param in method.parameters %}
Hi Paul, On 10/04/24 1:08 pm, Paul Elder wrote: > On Tue, Apr 09, 2024 at 06:24:28PM +0300, Laurent Pinchart wrote: >> Hi Paul, >> >> Thank you for the patch. >> >> On Tue, Apr 09, 2024 at 07:40:44PM +0900, Paul Elder wrote: >>> If an event function is defined with no parameters, there would be a >>> compilation error complaining about unused parameters in the generated >>> code for the data and dataSize parameters that would normally correspond >>> to serialized data. Fix this by simply marking the parameters as >>> maybe_unused. >>> >>> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> >>> --- >>> .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >>> index c37c4941..238cf4a5 100644 >>> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >>> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl >>> @@ -235,8 +235,8 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) >>> } >>> >>> void {{proxy_name}}::{{method.mojom_name}}IPC( >>> - std::vector<uint8_t>::const_iterator data, >>> - size_t dataSize, >>> + [[maybe_unused]] std::vector<uint8_t>::const_iterator data, >>> + [[maybe_unused]] size_t dataSize, >> Do you think it would be useful to set [[maybe_unused]] only when there >> are no parameters, or would that be overkill ? I suppose it's called >> "maybe_unused" and ont "unused" for a reason, so > I think it's overkill :) > >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Umang Jain <umang.jain@ideasonboard.com> > > Thanks, > > Paul > >>> [[maybe_unused]] const std::vector<SharedFD> &fds) >>> { >>> {%- for param in method.parameters %}
Hi Paul, On Tue, Apr 09, 2024 at 07:40:44PM +0900, Paul Elder wrote: > If an event function is defined with no parameters, there would be a > compilation error complaining about unused parameters in the generated > code for the data and dataSize parameters that would normally correspond > to serialized data. Fix this by simply marking the parameters as > maybe_unused. There's another similar issue when using async methods with no parameters in the IPA interface. It can be reproduced with the following test: diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom index dd991f7e70c9..28dfd878ea39 100644 --- a/include/libcamera/ipa/vimc.mojom +++ b/include/libcamera/ipa/vimc.mojom @@ -48,6 +48,7 @@ interface IPAVimcInterface { * handle parameters at runtime. */ [async] fillParamsBuffer(uint32 frame, uint32 bufferId); + [async] test(); }; interface IPAVimcEventInterface { diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp index 2c255778990a..e62cf9f22089 100644 --- a/src/ipa/vimc/vimc.cpp +++ b/src/ipa/vimc/vimc.cpp @@ -49,6 +49,8 @@ public: void queueRequest(uint32_t frame, const ControlList &controls) override; void fillParamsBuffer(uint32_t frame, uint32_t bufferId) override; + void test(); + private: void initTrace(); void trace(enum ipa::vimc::IPAOperationCode operation); The result is the following compilation error: src/libcamera/proxy/vimc_ipa_proxy.cpp: In member function ‘void libcamera::ipa::vimc::IPAProxyVimc::testThread()’: src/libcamera/proxy/vimc_ipa_proxy.cpp:537:70: error: expected primary-expression before ‘)’ token 537 | proxy_.invokeMethod(&ThreadProxy::test, ConnectionTypeQueued,); | > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > index c37c4941..238cf4a5 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl > @@ -235,8 +235,8 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) > } > > void {{proxy_name}}::{{method.mojom_name}}IPC( > - std::vector<uint8_t>::const_iterator data, > - size_t dataSize, > + [[maybe_unused]] std::vector<uint8_t>::const_iterator data, > + [[maybe_unused]] size_t dataSize, > [[maybe_unused]] const std::vector<SharedFD> &fds) > { > {%- for param in method.parameters %}
diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl index c37c4941..238cf4a5 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy.cpp.tmpl @@ -235,8 +235,8 @@ void {{proxy_name}}::recvMessage(const IPCMessage &data) } void {{proxy_name}}::{{method.mojom_name}}IPC( - std::vector<uint8_t>::const_iterator data, - size_t dataSize, + [[maybe_unused]] std::vector<uint8_t>::const_iterator data, + [[maybe_unused]] size_t dataSize, [[maybe_unused]] const std::vector<SharedFD> &fds) { {%- for param in method.parameters %}
If an event function is defined with no parameters, there would be a compilation error complaining about unused parameters in the generated code for the data and dataSize parameters that would normally correspond to serialized data. Fix this by simply marking the parameters as maybe_unused. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- .../generators/libcamera_templates/module_ipa_proxy.cpp.tmpl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)