Groups keyboard shortcuts have been updated
Dismiss
See shortcuts

[PATCH] parser: libconfig: allow implicit conversion from INT to INT64

9 views
Skip to first unread message

Christian Eggers

unread,
Jul 12, 2024, 3:58:56 AM7/12/24
to swup...@googlegroups.com, stefan...@swupdate.org, Christian Eggers
Since 1db0aefe57de ("Enforce type check in sw-description"),
GET_FIELD_INT64() does not assign a config value anymore if the type
detected by libconfig differs. This type check needs to be slightly
relaxed to allow assignment of parsed INT32 values when a INT64 is
expected. Otherwise this would require conversion of the sw-description
in the .swu files which breaks compatiblity with existing update files.

Link: https://groups.google.com/g/swupdate/c/UeALEHCAusQ
Signed-off-by: Christian Eggers <ceg...@arri.de>
---
corelib/parsing_library_libconfig.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/corelib/parsing_library_libconfig.c b/corelib/parsing_library_libconfig.c
index ddb79f6fb152..727697448ac4 100644
--- a/corelib/parsing_library_libconfig.c
+++ b/corelib/parsing_library_libconfig.c
@@ -42,8 +42,13 @@ static unsigned int map_field_type(field_type_t type)
static void get_value_libconfig(const config_setting_t *e, void *dest, field_type_t expected_type)
{
int type = config_setting_type(e);
- if (type != map_field_type(expected_type))
- return;
+ if (type != map_field_type(expected_type)) {
+ /* Only allow implicit conversion from INT to INT64 */
+ if (type == CONFIG_TYPE_INT && expected_type == TYPE_INT64)
+ type = CONFIG_TYPE_INT64;
+ else
+ return;
+ }
switch (type) {
case CONFIG_TYPE_INT:
*(int *)dest = config_setting_get_int(e);
--
2.44.1

Stefano Babic

unread,
Jul 12, 2024, 4:11:20 AM7/12/24
to Christian Eggers, swup...@googlegroups.com, stefan...@swupdate.org
Hi Christian,
There is no conversion, the type is forced to be INT64 and just solves
your use case. It will be plausible to call GET_FIELD_INT in the code,
and a user has written in sw-description something like
attribute = 1234L;

If this should be solved, it should be complete in both directions.

Best regards,
Stefano

Christian Eggers

unread,
Jul 12, 2024, 7:12:26 AM7/12/24
to swup...@googlegroups.com, Stefano Babic
Hi Stefano,
This would obviously weaken the type safety requirements you just introduced.
Maybe this could be postponed until somebody actually complains?
How should INT64 values which don't fit into INT32 be handled?

I also think that it is problematic that get_value_libconfig() simply
does nothing if there is a type mismatch. I would prefer printing at
least a warning that there was a problem parsing the sw-description.
Otherwise the problem hits in somewhere else (e.g. deleting UBI volumes
in my case) and requires using a debugger for finding the source.

Best regards,
Christian





Stefano Babic

unread,
Jul 12, 2024, 7:26:50 AM7/12/24
to Christian Eggers, swup...@googlegroups.com, Stefano Babic
For int and int64, yes. But I do not see how we can have both and
maintain compatibility. Or we say well, old SWUs must be updated and
resigned.

> Maybe this could be postponed until somebody actually complains?

In my experience, this means never and it remains unsolved until there
is another breakage and then a new work-around is searched.

> How should INT64 values which don't fit into INT32 be handled?

The master is SWUpdate internal type, and it decides which is the type
to be internal stored. The case can be detected because libconfig is
reporting the type. In such case, a WARN should be raised and the parser
does its best. But it can be also detected if INT64 fit or not, and in
the second case an ERROR could be raised and parser stops.

>
> I also think that it is problematic that get_value_libconfig() simply
> does nothing if there is a type mismatch. I would prefer printing at
> least a warning that there was a problem parsing the sw-description.

Agree, a warning should be raised in mismatch.

> Otherwise the problem hits in somewhere else (e.g. deleting UBI volumes
> in my case) and requires using a debugger for finding the source.
>

Best regards,
Stefano

Reply all
Reply to author
Forward
0 new messages