| Message ID | 20210423104711.401547-3-paul.elder@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 078fbff8f41fbf5ab9d8389e63aa5d0dbbc1d5f5 |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Fri, Apr 23, 2021 at 07:47:10PM +0900, Paul Elder wrote: > Structs defined in mojom previously used the namespace of the mojom file > that was being used as the source. This is obviously not the correct > namespace for structs that are defined in core.mojom. Fix the jinja > function for getting the element type including namespace, and use it. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> Nice cleanup. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > New in v3 > --- > .../module_ipa_serializer.h.tmpl | 2 +- > .../libcamera_templates/serializer.tmpl | 34 +++++++++---------- > .../generators/mojom_libcamera_generator.py | 16 ++++++--- > 3 files changed, 30 insertions(+), 22 deletions(-) > > diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl > index 64ae99dc..779d2114 100644 > --- a/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl > +++ b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl > @@ -30,7 +30,7 @@ namespace libcamera { > LOG_DECLARE_CATEGORY(IPADataSerializer) > {% for struct in structs_nonempty %} > template<> > -class IPADataSerializer<{{struct|name_full(namespace_str)}}> > +class IPADataSerializer<{{struct|name_full}}> > { > public: > {{- serializer.serializer(struct, namespace_str)}} > diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl > index af35b9e3..d8d55807 100644 > --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl > +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl > @@ -68,7 +68,7 @@ > {%- elif field|is_str %} > IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}); > {%- else %} > - IPADataSerializer<{{field|name_full(namespace)}}>::serialize(data.{{field.mojom_name}}, cs); > + IPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}}, cs); > {%- endif %} > appendPOD<uint32_t>(retData, {{field.mojom_name}}.size()); > {%- if field|has_fd %} > @@ -97,7 +97,7 @@ > {%- if field|is_pod %} > ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}); > {%- else %} > - ret.{{field.mojom_name}} = static_cast<{{field|name_full(namespace)}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}})); > + ret.{{field.mojom_name}} = static_cast<{{field|name_full}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}})); > {%- endif %} > {%- if not loop.last %} > m += {{field_size}}; > @@ -150,11 +150,11 @@ > {%- elif field|has_fd and (field|is_array or field|is_map) %} > IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs); > {%- elif field|has_fd and (not (field|is_array or field|is_map)) %} > - IPADataSerializer<{{field|name_full(namespace)}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs); > + IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs); > {%- elif (not field|has_fd) and (field|is_array or field|is_map) %} > IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, cs); > {%- else %} > - IPADataSerializer<{{field|name_full(namespace)}}>::deserialize(m, m + {{field.mojom_name}}Size, cs); > + IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, cs); > {%- endif %} > {%- if not loop.last %} > m += {{field_size}}; > @@ -178,7 +178,7 @@ > #} > {%- macro serializer(struct, namespace) %} > static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> > - serialize(const {{struct|name_full(namespace)}} &data, > + serialize(const {{struct|name_full}} &data, > {%- if struct|needs_control_serializer %} > ControlSerializer *cs) > {%- else %} > @@ -208,7 +208,7 @@ > # \a struct, in the case that \a struct has file descriptors. > #} > {%- macro deserializer_fd(struct, namespace) %} > - static {{struct|name_full(namespace)}} > + static {{struct|name_full}} > deserialize(std::vector<uint8_t> &data, > std::vector<int32_t> &fds, > {%- if struct|needs_control_serializer %} > @@ -217,11 +217,11 @@ > ControlSerializer *cs = nullptr) > {%- endif %} > { > - return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs); > + return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs); > } > > {# \todo Don't inline this function #} > - static {{struct|name_full(namespace)}} > + static {{struct|name_full}} > deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > std::vector<int32_t>::const_iterator fdsBegin, > @@ -232,7 +232,7 @@ > [[maybe_unused]] ControlSerializer *cs = nullptr) > {%- endif %} > { > - {{struct|name_full(namespace)}} ret; > + {{struct|name_full}} ret; > std::vector<uint8_t>::const_iterator m = dataBegin; > std::vector<int32_t>::const_iterator n = fdsBegin; > > @@ -253,22 +253,22 @@ > # deserializers with file descriptors. > #} > {%- macro deserializer_fd_simple(struct, namespace) %} > - static {{struct|name_full(namespace)}} > + static {{struct|name_full}} > deserialize(std::vector<uint8_t> &data, > [[maybe_unused]] std::vector<int32_t> &fds, > ControlSerializer *cs = nullptr) > { > - return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs); > + return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs); > } > > - static {{struct|name_full(namespace)}} > + static {{struct|name_full}} > deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, > [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, > ControlSerializer *cs = nullptr) > { > - return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(dataBegin, dataEnd, cs); > + return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs); > } > {%- endmacro %} > > @@ -280,7 +280,7 @@ > # \a struct, in the case that \a struct does not have file descriptors. > #} > {%- macro deserializer_no_fd(struct, namespace) %} > - static {{struct|name_full(namespace)}} > + static {{struct|name_full}} > deserialize(std::vector<uint8_t> &data, > {%- if struct|needs_control_serializer %} > ControlSerializer *cs) > @@ -288,11 +288,11 @@ > ControlSerializer *cs = nullptr) > {%- endif %} > { > - return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs); > + return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs); > } > > {# \todo Don't inline this function #} > - static {{struct|name_full(namespace)}} > + static {{struct|name_full}} > deserialize(std::vector<uint8_t>::const_iterator dataBegin, > std::vector<uint8_t>::const_iterator dataEnd, > {%- if struct|needs_control_serializer %} > @@ -301,7 +301,7 @@ > [[maybe_unused]] ControlSerializer *cs = nullptr) > {%- endif %} > { > - {{struct|name_full(namespace)}} ret; > + {{struct|name_full}} ret; > std::vector<uint8_t>::const_iterator m = dataBegin; > > size_t dataSize = std::distance(dataBegin, dataEnd); > diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py > index db9e28a6..effdfed6 100644 > --- a/utils/ipc/generators/mojom_libcamera_generator.py > +++ b/utils/ipc/generators/mojom_libcamera_generator.py > @@ -258,12 +258,12 @@ def GetNameForElement(element): > return element.mojom_name > # vectors > if (mojom.IsArrayKind(element)): > - elem_name = GetNameForElement(element.kind) > + elem_name = GetFullNameForElement(element.kind) > return f'std::vector<{elem_name}>' > # maps > if (mojom.IsMapKind(element)): > - key_name = GetNameForElement(element.key_kind) > - value_name = GetNameForElement(element.value_kind) > + key_name = GetFullNameForElement(element.key_kind) > + value_name = GetFullNameForElement(element.value_kind) > return f'std::map<{key_name}, {value_name}>' > # struct fields and function parameters > if isinstance(element, (mojom.Field, mojom.Method, mojom.Parameter)): > @@ -296,8 +296,16 @@ def GetNameForElement(element): > raise Exception('Unsupported element: %s' % element) > raise Exception('Unexpected element: %s' % element) > > -def GetFullNameForElement(element, namespace_str): > +def GetFullNameForElement(element): > name = GetNameForElement(element) > + namespace_str = '' > + if mojom.IsStructKind(element): > + namespace_str = element.module.mojom_namespace.replace('.', '::') > + elif (hasattr(element, 'kind') and > + (mojom.IsStructKind(element.kind) or > + mojom.IsEnumKind(element.kind))): > + namespace_str = element.kind.module.mojom_namespace.replace('.', '::') > + > if namespace_str == '': > return name > return f'{namespace_str}::{name}'
diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl index 64ae99dc..779d2114 100644 --- a/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl +++ b/utils/ipc/generators/libcamera_templates/module_ipa_serializer.h.tmpl @@ -30,7 +30,7 @@ namespace libcamera { LOG_DECLARE_CATEGORY(IPADataSerializer) {% for struct in structs_nonempty %} template<> -class IPADataSerializer<{{struct|name_full(namespace_str)}}> +class IPADataSerializer<{{struct|name_full}}> { public: {{- serializer.serializer(struct, namespace_str)}} diff --git a/utils/ipc/generators/libcamera_templates/serializer.tmpl b/utils/ipc/generators/libcamera_templates/serializer.tmpl index af35b9e3..d8d55807 100644 --- a/utils/ipc/generators/libcamera_templates/serializer.tmpl +++ b/utils/ipc/generators/libcamera_templates/serializer.tmpl @@ -68,7 +68,7 @@ {%- elif field|is_str %} IPADataSerializer<{{field|name}}>::serialize(data.{{field.mojom_name}}); {%- else %} - IPADataSerializer<{{field|name_full(namespace)}}>::serialize(data.{{field.mojom_name}}, cs); + IPADataSerializer<{{field|name_full}}>::serialize(data.{{field.mojom_name}}, cs); {%- endif %} appendPOD<uint32_t>(retData, {{field.mojom_name}}.size()); {%- if field|has_fd %} @@ -97,7 +97,7 @@ {%- if field|is_pod %} ret.{{field.mojom_name}} = IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field_size}}); {%- else %} - ret.{{field.mojom_name}} = static_cast<{{field|name_full(namespace)}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}})); + ret.{{field.mojom_name}} = static_cast<{{field|name_full}}>(IPADataSerializer<uint{{field|bit_width}}_t>::deserialize(m, m + {{field_size}})); {%- endif %} {%- if not loop.last %} m += {{field_size}}; @@ -150,11 +150,11 @@ {%- elif field|has_fd and (field|is_array or field|is_map) %} IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs); {%- elif field|has_fd and (not (field|is_array or field|is_map)) %} - IPADataSerializer<{{field|name_full(namespace)}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs); + IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, n, n + {{field.mojom_name}}FdsSize, cs); {%- elif (not field|has_fd) and (field|is_array or field|is_map) %} IPADataSerializer<{{field|name}}>::deserialize(m, m + {{field.mojom_name}}Size, cs); {%- else %} - IPADataSerializer<{{field|name_full(namespace)}}>::deserialize(m, m + {{field.mojom_name}}Size, cs); + IPADataSerializer<{{field|name_full}}>::deserialize(m, m + {{field.mojom_name}}Size, cs); {%- endif %} {%- if not loop.last %} m += {{field_size}}; @@ -178,7 +178,7 @@ #} {%- macro serializer(struct, namespace) %} static std::tuple<std::vector<uint8_t>, std::vector<int32_t>> - serialize(const {{struct|name_full(namespace)}} &data, + serialize(const {{struct|name_full}} &data, {%- if struct|needs_control_serializer %} ControlSerializer *cs) {%- else %} @@ -208,7 +208,7 @@ # \a struct, in the case that \a struct has file descriptors. #} {%- macro deserializer_fd(struct, namespace) %} - static {{struct|name_full(namespace)}} + static {{struct|name_full}} deserialize(std::vector<uint8_t> &data, std::vector<int32_t> &fds, {%- if struct|needs_control_serializer %} @@ -217,11 +217,11 @@ ControlSerializer *cs = nullptr) {%- endif %} { - return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs); + return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), fds.cbegin(), fds.cend(), cs); } {# \todo Don't inline this function #} - static {{struct|name_full(namespace)}} + static {{struct|name_full}} deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, std::vector<int32_t>::const_iterator fdsBegin, @@ -232,7 +232,7 @@ [[maybe_unused]] ControlSerializer *cs = nullptr) {%- endif %} { - {{struct|name_full(namespace)}} ret; + {{struct|name_full}} ret; std::vector<uint8_t>::const_iterator m = dataBegin; std::vector<int32_t>::const_iterator n = fdsBegin; @@ -253,22 +253,22 @@ # deserializers with file descriptors. #} {%- macro deserializer_fd_simple(struct, namespace) %} - static {{struct|name_full(namespace)}} + static {{struct|name_full}} deserialize(std::vector<uint8_t> &data, [[maybe_unused]] std::vector<int32_t> &fds, ControlSerializer *cs = nullptr) { - return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs); + return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs); } - static {{struct|name_full(namespace)}} + static {{struct|name_full}} deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, [[maybe_unused]] std::vector<int32_t>::const_iterator fdsBegin, [[maybe_unused]] std::vector<int32_t>::const_iterator fdsEnd, ControlSerializer *cs = nullptr) { - return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(dataBegin, dataEnd, cs); + return IPADataSerializer<{{struct|name_full}}>::deserialize(dataBegin, dataEnd, cs); } {%- endmacro %} @@ -280,7 +280,7 @@ # \a struct, in the case that \a struct does not have file descriptors. #} {%- macro deserializer_no_fd(struct, namespace) %} - static {{struct|name_full(namespace)}} + static {{struct|name_full}} deserialize(std::vector<uint8_t> &data, {%- if struct|needs_control_serializer %} ControlSerializer *cs) @@ -288,11 +288,11 @@ ControlSerializer *cs = nullptr) {%- endif %} { - return IPADataSerializer<{{struct|name_full(namespace)}}>::deserialize(data.cbegin(), data.cend(), cs); + return IPADataSerializer<{{struct|name_full}}>::deserialize(data.cbegin(), data.cend(), cs); } {# \todo Don't inline this function #} - static {{struct|name_full(namespace)}} + static {{struct|name_full}} deserialize(std::vector<uint8_t>::const_iterator dataBegin, std::vector<uint8_t>::const_iterator dataEnd, {%- if struct|needs_control_serializer %} @@ -301,7 +301,7 @@ [[maybe_unused]] ControlSerializer *cs = nullptr) {%- endif %} { - {{struct|name_full(namespace)}} ret; + {{struct|name_full}} ret; std::vector<uint8_t>::const_iterator m = dataBegin; size_t dataSize = std::distance(dataBegin, dataEnd); diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py index db9e28a6..effdfed6 100644 --- a/utils/ipc/generators/mojom_libcamera_generator.py +++ b/utils/ipc/generators/mojom_libcamera_generator.py @@ -258,12 +258,12 @@ def GetNameForElement(element): return element.mojom_name # vectors if (mojom.IsArrayKind(element)): - elem_name = GetNameForElement(element.kind) + elem_name = GetFullNameForElement(element.kind) return f'std::vector<{elem_name}>' # maps if (mojom.IsMapKind(element)): - key_name = GetNameForElement(element.key_kind) - value_name = GetNameForElement(element.value_kind) + key_name = GetFullNameForElement(element.key_kind) + value_name = GetFullNameForElement(element.value_kind) return f'std::map<{key_name}, {value_name}>' # struct fields and function parameters if isinstance(element, (mojom.Field, mojom.Method, mojom.Parameter)): @@ -296,8 +296,16 @@ def GetNameForElement(element): raise Exception('Unsupported element: %s' % element) raise Exception('Unexpected element: %s' % element) -def GetFullNameForElement(element, namespace_str): +def GetFullNameForElement(element): name = GetNameForElement(element) + namespace_str = '' + if mojom.IsStructKind(element): + namespace_str = element.module.mojom_namespace.replace('.', '::') + elif (hasattr(element, 'kind') and + (mojom.IsStructKind(element.kind) or + mojom.IsEnumKind(element.kind))): + namespace_str = element.kind.module.mojom_namespace.replace('.', '::') + if namespace_str == '': return name return f'{namespace_str}::{name}'
Structs defined in mojom previously used the namespace of the mojom file that was being used as the source. This is obviously not the correct namespace for structs that are defined in core.mojom. Fix the jinja function for getting the element type including namespace, and use it. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- New in v3 --- .../module_ipa_serializer.h.tmpl | 2 +- .../libcamera_templates/serializer.tmpl | 34 +++++++++---------- .../generators/mojom_libcamera_generator.py | 16 ++++++--- 3 files changed, 30 insertions(+), 22 deletions(-)