| Message ID | 20200615082702.32693-5-sde@unmatched.eu |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add semantic versioning support | expand |
Hi Stjin, I get a new regression issue with the patchset: On 15.06.20 10:26, Stijn Devriendt wrote: > When the string is malformed, we'll treat it as 0.0.0 and emit a > warning. This matches the approach from the previous code, to > ignore incorrect version parts, ending up with a NUL-version. > > Signed-off-by: Stijn Devriendt <sde@unmatched.eu> > --- > core/artifacts_versions.c | 46 +++++++++++++++++++++++++++++++++++++++ > core/swupdate.c | 2 ++ > include/util.h | 1 + > parser/parser.c | 3 +++ > 4 files changed, 52 insertions(+) > > diff --git a/core/artifacts_versions.c b/core/artifacts_versions.c > index 6fe5d60..ccc283e 100644 > --- a/core/artifacts_versions.c > +++ b/core/artifacts_versions.c > @@ -25,6 +25,7 @@ > #include "swupdate.h" > #include "parselib.h" > #include "swupdate_settings.h" > +#include "semver.h" > > /* > * Read versions of components from a file, if provided > @@ -66,6 +67,9 @@ static int read_sw_version_file(struct swupdate_cfg *sw) > } > strlcpy(swcomp->name, name, sizeof(swcomp->name)); > strlcpy(swcomp->version, version, sizeof(swcomp->version)); > + > + cleanup_version(swcomp->version); > + Issue is exactly here. This requires that all versions are in the semantic format or as 4 integer. However, installed-if-different just check if version differs and users can set any kind of string. I got complain about this: install-if-different = true; version = "U-Boot_SPL_2019.04-imx_v2019.04_4.19.35_1.0.0+g85bdcc7981_(Jul_08_2020_-_07:07:43_+0000)"; Apart of the first price as longest version id (that is automatically generated), this is accepted up now by SWUpdate. However, you call cleanup_version() after parsing the version number. Is it this really required ? Cannot we move it inside the comparison ? int compare_versions(const char* left_version, const char* right_version) { if is_oldstyle_version ==> do old style if (!semver_parse(left_version, &left_sem) && !semver_parse(right_version, &right_sem); ==> semantic version else ==> as before, compare the twostrings. Now any string like the one above is removed and version is back to 0.0.0.0, and artefact is always installed. Best regards, Stefano Babic > LIST_INSERT_HEAD(&sw->installed_sw_list, swcomp, next); > TRACE("Installed %s: Version %s", > swcomp->name, > @@ -118,6 +122,8 @@ static int versions_settings(void *setting, void *data) > GET_FIELD_STRING(LIBCFG_PARSER, elem, "name", swcomp->name); > GET_FIELD_STRING(LIBCFG_PARSER, elem, "version", swcomp->version); > > + cleanup_version(swcomp->version); > + > LIST_INSERT_HEAD(&sw->installed_sw_list, swcomp, next); > TRACE("Installed %s: Version %s", > swcomp->name, > @@ -154,6 +160,46 @@ void get_sw_versions(char __attribute__ ((__unused__)) *cfgname, > } > #endif > > +static const char ACCEPTED_CHARS[] = "0123456789."; > + > +static bool is_oldstyle_version(const char* version_string) > +{ > + while (*version_string) > + { > + if (strchr(ACCEPTED_CHARS, *version_string) == NULL) > + return false; > + } > + return true; > +} > + > +void cleanup_version(char* str) > +{ > + semver_t version = {}; > + int res = semver_clean(str); > + > + /* This is only expected for overly long version strings. > + * Is SWUPDATE_GENERAL_STRING_SIZE > semver.c:MAX_SIZE + 1??? > + */ > + assert(res == 0); > + if (res == -1) { > + WARN("Version string too long. Using 0.0.0 instead!"); > + str[0] = '\0'; > + return; > + } > + > + if (is_oldstyle_version(str)) > + return; > + > + if (semver_parse(str, &version) == 0) { > + semver_render(&version, str); > + } else { > + WARN("Unparseable version string. Using 0.0.0 instead!"); > + str[0] = '\0'; > + } > + > + semver_free(&version); > +} > + > /* > * convert a version string into a number > * version string is in the format: > diff --git a/core/swupdate.c b/core/swupdate.c > index 74dfbbe..8e85127 100644 > --- a/core/swupdate.c > +++ b/core/swupdate.c > @@ -792,11 +792,13 @@ int main(int argc, char **argv) > swcfg.globals.no_downgrading = 1; > strlcpy(swcfg.globals.minimum_version, optarg, > sizeof(swcfg.globals.minimum_version)); > + cleanup_version(swcfg.globals.minimum_version); > break; > case 'R': > swcfg.globals.no_reinstalling = 1; > strlcpy(swcfg.globals.current_version, optarg, > sizeof(swcfg.globals.current_version)); > + cleanup_version(swcfg.globals.current_version); > break; > case 'M': > swcfg.globals.no_transaction_marker = 1; > diff --git a/include/util.h b/include/util.h > index 55fe70f..6a027c6 100644 > --- a/include/util.h > +++ b/include/util.h > @@ -216,6 +216,7 @@ size_t snescape(char *dst, size_t n, const char *src); > void freeargs (char **argv); > int get_hw_revision(struct hw_type *hw); > void get_sw_versions(char *cfgfname, struct swupdate_cfg *sw); > +void cleanup_version(char* str); > __u64 version_to_number(const char *version_string); > int hwid_match(const char* rev, const char* hwrev); > int check_hw_compatibility(struct swupdate_cfg *cfg); > diff --git a/parser/parser.c b/parser/parser.c > index 31efe46..981d40d 100644 > --- a/parser/parser.c > +++ b/parser/parser.c > @@ -273,6 +273,9 @@ static int parse_common_attributes(parsertype p, void *elem, struct img_type *im > > GET_FIELD_STRING(p, elem, "name", image->id.name); > GET_FIELD_STRING(p, elem, "version", image->id.version); > + > + cleanup_version(image->id.version); > + > GET_FIELD_STRING(p, elem, "filename", image->fname); > GET_FIELD_STRING(p, elem, "path", image->path); > GET_FIELD_STRING(p, elem, "volume", image->volname); >
On Thursday, July 9, 2020 at 5:41:27 PM UTC+2, Stefano Babic wrote: > > Hi Stjin, > > I get a new regression issue with the patchset: > > On 15.06.20 10:26, Stijn Devriendt wrote: > > When the string is malformed, we'll treat it as 0.0.0 and emit a > > warning. This matches the approach from the previous code, to > > ignore incorrect version parts, ending up with a NUL-version. > > > > Signed-off-by: Stijn Devriendt <s...@unmatched.eu <javascript:>> > > --- > > core/artifacts_versions.c | 46 +++++++++++++++++++++++++++++++++++++++ > > core/swupdate.c | 2 ++ > > include/util.h | 1 + > > parser/parser.c | 3 +++ > > 4 files changed, 52 insertions(+) > > > > diff --git a/core/artifacts_versions.c b/core/artifacts_versions.c > > index 6fe5d60..ccc283e 100644 > > --- a/core/artifacts_versions.c > > +++ b/core/artifacts_versions.c > > @@ -25,6 +25,7 @@ > > #include "swupdate.h" > > #include "parselib.h" > > #include "swupdate_settings.h" > > +#include "semver.h" > > > > /* > > * Read versions of components from a file, if provided > > @@ -66,6 +67,9 @@ static int read_sw_version_file(struct swupdate_cfg > *sw) > > } > > strlcpy(swcomp->name, name, > sizeof(swcomp->name)); > > strlcpy(swcomp->version, version, > sizeof(swcomp->version)); > > + > > + cleanup_version(swcomp->version); > > + > > Issue is exactly here. This requires that all versions are in the > semantic format or as 4 integer. However, installed-if-different just > check if version differs and users can set any kind of string. > > I got complain about this: > > install-if-different = true; > version = > "U-Boot_SPL_2019.04-imx_v2019.04_4.19.35_1.0.0+g85bdcc7981_(Jul_08_2020_-_07:07:43_+0000)"; > > > > Apart of the first price as longest version id (that is automatically > generated), this is accepted up now by SWUpdate. > > However, you call cleanup_version() after parsing the version number. Is > it this really required ? Cannot we move it inside the comparison ? > > int compare_versions(const char* left_version, const char* right_version) > { > if is_oldstyle_version > ==> do old style > > if (!semver_parse(left_version, &left_sem) && > !semver_parse(right_version, &right_sem); > > ==> semantic version > > else > ==> as before, compare the twostrings. > > Now any string like the one above is removed and version is back to > 0.0.0.0, and artefact is always installed. > > Hi Stefano, That is indeed a regression. However, the problem I see with the proposed solution is that version comparison becomes dark magic. SWUpdate will print the version strings before calling cleanup_version(). The user will be unaware from the logs/traces that what we are actually comparing is different from what the user is expecting we are comparing. I believe that the user must be able to see the in the logs what we are actually comparing. If I understand the code correctly, we could get do this if: - in core/swupdate.c - main(): remove the cleanup_version call during argument parsing for the 'R' case i.e. no-reinstall will not check the version passed for the 'current_version' string. This leaves it in place for the 'N' case, i.e. no-downgrading. - in parser/parser.c - parse_common_attributes(): make the cleanup_version call conditional on install-if-higher being specified I also think I forgot a case when parsing the config file: - in core/swupdate.c - read_globals_settings(): add a check on minimum_version after setting the no_downgrading flag. What do you think? Regards, Stijn > Best regards, > Stefano Babic > > > LIST_INSERT_HEAD(&sw->installed_sw_list, > swcomp, next); > > TRACE("Installed %s: Version %s", > > swcomp->name, > > @@ -118,6 +122,8 @@ static int versions_settings(void *setting, void > *data) > > GET_FIELD_STRING(LIBCFG_PARSER, elem, "name", > swcomp->name); > > GET_FIELD_STRING(LIBCFG_PARSER, elem, "version", > swcomp->version); > > > > + cleanup_version(swcomp->version); > > + > > LIST_INSERT_HEAD(&sw->installed_sw_list, swcomp, next); > > TRACE("Installed %s: Version %s", > > swcomp->name, > > @@ -154,6 +160,46 @@ void get_sw_versions(char __attribute__ > ((__unused__)) *cfgname, > > } > > #endif > > > > +static const char ACCEPTED_CHARS[] = "0123456789."; > > + > > +static bool is_oldstyle_version(const char* version_string) > > +{ > > + while (*version_string) > > + { > > + if (strchr(ACCEPTED_CHARS, *version_string) == NULL) > > + return false; > > + } > > + return true; > > +} > > + > > +void cleanup_version(char* str) > > +{ > > + semver_t version = {}; > > + int res = semver_clean(str); > > + > > + /* This is only expected for overly long version strings. > > + * Is SWUPDATE_GENERAL_STRING_SIZE > semver.c:MAX_SIZE + 1??? > > + */ > > + assert(res == 0); > > + if (res == -1) { > > + WARN("Version string too long. Using 0.0.0 instead!"); > > + str[0] = '\0'; > > + return; > > + } > > + > > + if (is_oldstyle_version(str)) > > + return; > > + > > + if (semver_parse(str, &version) == 0) { > > + semver_render(&version, str); > > + } else { > > + WARN("Unparseable version string. Using 0.0.0 > instead!"); > > + str[0] = '\0'; > > + } > > + > > + semver_free(&version); > > +} > > + > > /* > > * convert a version string into a number > > * version string is in the format: > > diff --git a/core/swupdate.c b/core/swupdate.c > > index 74dfbbe..8e85127 100644 > > --- a/core/swupdate.c > > +++ b/core/swupdate.c > > @@ -792,11 +792,13 @@ int main(int argc, char **argv) > > swcfg.globals.no_downgrading = 1; > > strlcpy(swcfg.globals.minimum_version, optarg, > > sizeof(swcfg.globals.minimum_version)); > > + cleanup_version(swcfg.globals.minimum_version); > > break; > > case 'R': > > swcfg.globals.no_reinstalling = 1; > > strlcpy(swcfg.globals.current_version, optarg, > > sizeof(swcfg.globals.current_version)); > > + cleanup_version(swcfg.globals.current_version); > > break; > > case 'M': > > swcfg.globals.no_transaction_marker = 1; > > diff --git a/include/util.h b/include/util.h > > index 55fe70f..6a027c6 100644 > > --- a/include/util.h > > +++ b/include/util.h > > @@ -216,6 +216,7 @@ size_t snescape(char *dst, size_t n, const char > *src); > > void freeargs (char **argv); > > int get_hw_revision(struct hw_type *hw); > > void get_sw_versions(char *cfgfname, struct swupdate_cfg *sw); > > +void cleanup_version(char* str); > > __u64 version_to_number(const char *version_string); > > int hwid_match(const char* rev, const char* hwrev); > > int check_hw_compatibility(struct swupdate_cfg *cfg); > > diff --git a/parser/parser.c b/parser/parser.c > > index 31efe46..981d40d 100644 > > --- a/parser/parser.c > > +++ b/parser/parser.c > > @@ -273,6 +273,9 @@ static int parse_common_attributes(parsertype p, > void *elem, struct img_type *im > > > > GET_FIELD_STRING(p, elem, "name", image->id.name); > > GET_FIELD_STRING(p, elem, "version", image->id.version); > > + > > + cleanup_version(image->id.version); > > + > > GET_FIELD_STRING(p, elem, "filename", image->fname); > > GET_FIELD_STRING(p, elem, "path", image->path); > > GET_FIELD_STRING(p, elem, "volume", image->volname); > > > > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de > <javascript:> > ===================================================================== >
Hi Stjin, On 13.07.20 10:57, highguy@gmail.com wrote: > On Thursday, July 9, 2020 at 5:41:27 PM UTC+2, Stefano Babic wrote: > > Hi Stjin, > > I get a new regression issue with the patchset: > > On 15.06.20 10:26, Stijn Devriendt wrote: > > When the string is malformed, we'll treat it as 0.0.0 and emit a > > warning. This matches the approach from the previous code, to > > ignore incorrect version parts, ending up with a NUL-version. > > > > Signed-off-by: Stijn Devriendt <s...@unmatched.eu <javascript:>> > > --- > > core/artifacts_versions.c | 46 > +++++++++++++++++++++++++++++++++++++++ > > core/swupdate.c | 2 ++ > > include/util.h | 1 + > > parser/parser.c | 3 +++ > > 4 files changed, 52 insertions(+) > > > > diff --git a/core/artifacts_versions.c b/core/artifacts_versions.c > > index 6fe5d60..ccc283e 100644 > > --- a/core/artifacts_versions.c > > +++ b/core/artifacts_versions.c > > @@ -25,6 +25,7 @@ > > #include "swupdate.h" > > #include "parselib.h" > > #include "swupdate_settings.h" > > +#include "semver.h" > > > > /* > > * Read versions of components from a file, if provided > > @@ -66,6 +67,9 @@ static int read_sw_version_file(struct > swupdate_cfg *sw) > > } > > strlcpy(swcomp->name, name, > sizeof(swcomp->name)); > > strlcpy(swcomp->version, version, > sizeof(swcomp->version)); > > + > > + cleanup_version(swcomp->version); > > + > > Issue is exactly here. This requires that all versions are in the > semantic format or as 4 integer. However, installed-if-different just > check if version differs and users can set any kind of string. > > I got complain about this: > > install-if-different = true; > version = > "U-Boot_SPL_2019.04-imx_v2019.04_4.19.35_1.0.0+g85bdcc7981_(Jul_08_2020_-_07:07:43_+0000)"; > > > > Apart of the first price as longest version id (that is automatically > generated), this is accepted up now by SWUpdate. > > However, you call cleanup_version() after parsing the version > number. Is > it this really required ? Cannot we move it inside the comparison ? > > int compare_versions(const char* left_version, const char* > right_version) > { > if is_oldstyle_version > ==> do old style > > if (!semver_parse(left_version, &left_sem) && > !semver_parse(right_version, &right_sem); > > ==> semantic version > > else > ==> as before, compare the twostrings. > > Now any string like the one above is removed and version is back to > 0.0.0.0, and artefact is always installed. > > > Hi Stefano, > > That is indeed a regression. > Right. > However, the problem I see with the proposed solution is that version > comparison becomes > dark magic. SWUpdate will print the version strings before calling > cleanup_version(). We can print the versions in compare_versions, then we have exactly the values that are compared. > The user will be unaware from the logs/traces that what we are actually > comparing is different > from what the user is expecting we are comparing. Yes, we should avoid this. > > I believe that the user must be able to see the in the logs what we are > actually comparing. > If I understand the code correctly, we could get do this if: > - in core/swupdate.c - main(): remove the cleanup_version call during > argument parsing for the 'R' case We can, but as this was just added to let reinstall the same version that currently runs. > i.e. no-reinstall will not check the version passed for the > 'current_version' string. > This leaves it in place for the 'N' case, i.e. no-downgrading. > - in parser/parser.c - parse_common_attributes(): make the > cleanup_version call conditional on > install-if-higher being specified We could do this - semantic version has meaning just in case we want to check if a version is greater. For strict comparison wiht install-if different, it is not needed. > > I also think I forgot a case when parsing the config file: > - in core/swupdate.c - read_globals_settings(): add a check on > minimum_version after setting > the no_downgrading flag. > Looks plausible - I have attached a first draft according to my previous e-mail. Feel free to change it or drop it completely. Best regards, Stefano Babic > What do you think? > > Regards, > Stijn > > > > Best regards, > Stefano Babic > > > LIST_INSERT_HEAD(&sw->installed_sw_list, > swcomp, next); > > TRACE("Installed %s: Version %s", > > swcomp->name, > > @@ -118,6 +122,8 @@ static int versions_settings(void *setting, > void *data) > > GET_FIELD_STRING(LIBCFG_PARSER, elem, "name", > swcomp->name); > > GET_FIELD_STRING(LIBCFG_PARSER, elem, "version", > swcomp->version); > > > > + cleanup_version(swcomp->version); > > + > > LIST_INSERT_HEAD(&sw->installed_sw_list, swcomp, > next); > > TRACE("Installed %s: Version %s", > > swcomp->name, > > @@ -154,6 +160,46 @@ void get_sw_versions(char __attribute__ > ((__unused__)) *cfgname, > > } > > #endif > > > > +static const char ACCEPTED_CHARS[] = "0123456789."; > > + > > +static bool is_oldstyle_version(const char* version_string) > > +{ > > + while (*version_string) > > + { > > + if (strchr(ACCEPTED_CHARS, *version_string) == NULL) > > + return false; > > + } > > + return true; > > +} > > + > > +void cleanup_version(char* str) > > +{ > > + semver_t version = {}; > > + int res = semver_clean(str); > > + > > + /* This is only expected for overly long version strings. > > + * Is SWUPDATE_GENERAL_STRING_SIZE > semver.c:MAX_SIZE + > 1??? > > + */ > > + assert(res == 0); > > + if (res == -1) { > > + WARN("Version string too long. Using 0.0.0 > instead!"); > > + str[0] = '\0'; > > + return; > > + } > > + > > + if (is_oldstyle_version(str)) > > + return; > > + > > + if (semver_parse(str, &version) == 0) { > > + semver_render(&version, str); > > + } else { > > + WARN("Unparseable version string. Using 0.0.0 > instead!"); > > + str[0] = '\0'; > > + } > > + > > + semver_free(&version); > > +} > > + > > /* > > * convert a version string into a number > > * version string is in the format: > > diff --git a/core/swupdate.c b/core/swupdate.c > > index 74dfbbe..8e85127 100644 > > --- a/core/swupdate.c > > +++ b/core/swupdate.c > > @@ -792,11 +792,13 @@ int main(int argc, char **argv) > > swcfg.globals.no_downgrading = 1; > > strlcpy(swcfg.globals.minimum_version, > optarg, > > > sizeof(swcfg.globals.minimum_version)); > > > > + cleanup_version(swcfg.globals.minimum_version); > > > break; > > case 'R': > > swcfg.globals.no_reinstalling = 1; > > strlcpy(swcfg.globals.current_version, > optarg, > > > sizeof(swcfg.globals.current_version)); > > > > + cleanup_version(swcfg.globals.current_version); > > > break; > > case 'M': > > swcfg.globals.no_transaction_marker = 1; > > diff --git a/include/util.h b/include/util.h > > index 55fe70f..6a027c6 100644 > > --- a/include/util.h > > +++ b/include/util.h > > @@ -216,6 +216,7 @@ size_t snescape(char *dst, size_t n, const > char *src); > > void freeargs (char **argv); > > int get_hw_revision(struct hw_type *hw); > > void get_sw_versions(char *cfgfname, struct swupdate_cfg *sw); > > +void cleanup_version(char* str); > > __u64 version_to_number(const char *version_string); > > int hwid_match(const char* rev, const char* hwrev); > > int check_hw_compatibility(struct swupdate_cfg *cfg); > > diff --git a/parser/parser.c b/parser/parser.c > > index 31efe46..981d40d 100644 > > --- a/parser/parser.c > > +++ b/parser/parser.c > > @@ -273,6 +273,9 @@ static int parse_common_attributes(parsertype > p, void *elem, struct img_type *im > > > > GET_FIELD_STRING(p, elem, "name", image->id.name > <http://id.name>); > > GET_FIELD_STRING(p, elem, "version", image->id.version); > > + > > + cleanup_version(image->id.version); > > + > > GET_FIELD_STRING(p, elem, "filename", image->fname); > > GET_FIELD_STRING(p, elem, "path", image->path); > > GET_FIELD_STRING(p, elem, "volume", image->volname); > > > > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > sba...@denx.de <javascript:> > ===================================================================== > > -- > You received this message because you are subscribed to the Google > Groups "swupdate" group. > To unsubscribe from this group and stop receiving emails from it, send > an email to swupdate+unsubscribe@googlegroups.com > <mailto:swupdate+unsubscribe@googlegroups.com>. > To view this discussion on the web visit > https://groups.google.com/d/msgid/swupdate/7e6f5940-0234-499c-9a43-ce8b7607ed9fo%40googlegroups.com > <https://groups.google.com/d/msgid/swupdate/7e6f5940-0234-499c-9a43-ce8b7607ed9fo%40googlegroups.com?utm_medium=email&utm_source=footer>.
diff --git a/core/artifacts_versions.c b/core/artifacts_versions.c index 6fe5d60..ccc283e 100644 --- a/core/artifacts_versions.c +++ b/core/artifacts_versions.c @@ -25,6 +25,7 @@ #include "swupdate.h" #include "parselib.h" #include "swupdate_settings.h" +#include "semver.h" /* * Read versions of components from a file, if provided @@ -66,6 +67,9 @@ static int read_sw_version_file(struct swupdate_cfg *sw) } strlcpy(swcomp->name, name, sizeof(swcomp->name)); strlcpy(swcomp->version, version, sizeof(swcomp->version)); + + cleanup_version(swcomp->version); + LIST_INSERT_HEAD(&sw->installed_sw_list, swcomp, next); TRACE("Installed %s: Version %s", swcomp->name, @@ -118,6 +122,8 @@ static int versions_settings(void *setting, void *data) GET_FIELD_STRING(LIBCFG_PARSER, elem, "name", swcomp->name); GET_FIELD_STRING(LIBCFG_PARSER, elem, "version", swcomp->version); + cleanup_version(swcomp->version); + LIST_INSERT_HEAD(&sw->installed_sw_list, swcomp, next); TRACE("Installed %s: Version %s", swcomp->name, @@ -154,6 +160,46 @@ void get_sw_versions(char __attribute__ ((__unused__)) *cfgname, } #endif +static const char ACCEPTED_CHARS[] = "0123456789."; + +static bool is_oldstyle_version(const char* version_string) +{ + while (*version_string) + { + if (strchr(ACCEPTED_CHARS, *version_string) == NULL) + return false; + } + return true; +} + +void cleanup_version(char* str) +{ + semver_t version = {}; + int res = semver_clean(str); + + /* This is only expected for overly long version strings. + * Is SWUPDATE_GENERAL_STRING_SIZE > semver.c:MAX_SIZE + 1??? + */ + assert(res == 0); + if (res == -1) { + WARN("Version string too long. Using 0.0.0 instead!"); + str[0] = '\0'; + return; + } + + if (is_oldstyle_version(str)) + return; + + if (semver_parse(str, &version) == 0) { + semver_render(&version, str); + } else { + WARN("Unparseable version string. Using 0.0.0 instead!"); + str[0] = '\0'; + } + + semver_free(&version); +} + /* * convert a version string into a number * version string is in the format: diff --git a/core/swupdate.c b/core/swupdate.c index 74dfbbe..8e85127 100644 --- a/core/swupdate.c +++ b/core/swupdate.c @@ -792,11 +792,13 @@ int main(int argc, char **argv) swcfg.globals.no_downgrading = 1; strlcpy(swcfg.globals.minimum_version, optarg, sizeof(swcfg.globals.minimum_version)); + cleanup_version(swcfg.globals.minimum_version); break; case 'R': swcfg.globals.no_reinstalling = 1; strlcpy(swcfg.globals.current_version, optarg, sizeof(swcfg.globals.current_version)); + cleanup_version(swcfg.globals.current_version); break; case 'M': swcfg.globals.no_transaction_marker = 1; diff --git a/include/util.h b/include/util.h index 55fe70f..6a027c6 100644 --- a/include/util.h +++ b/include/util.h @@ -216,6 +216,7 @@ size_t snescape(char *dst, size_t n, const char *src); void freeargs (char **argv); int get_hw_revision(struct hw_type *hw); void get_sw_versions(char *cfgfname, struct swupdate_cfg *sw); +void cleanup_version(char* str); __u64 version_to_number(const char *version_string); int hwid_match(const char* rev, const char* hwrev); int check_hw_compatibility(struct swupdate_cfg *cfg); diff --git a/parser/parser.c b/parser/parser.c index 31efe46..981d40d 100644 --- a/parser/parser.c +++ b/parser/parser.c @@ -273,6 +273,9 @@ static int parse_common_attributes(parsertype p, void *elem, struct img_type *im GET_FIELD_STRING(p, elem, "name", image->id.name); GET_FIELD_STRING(p, elem, "version", image->id.version); + + cleanup_version(image->id.version); + GET_FIELD_STRING(p, elem, "filename", image->fname); GET_FIELD_STRING(p, elem, "path", image->path); GET_FIELD_STRING(p, elem, "volume", image->volname);
When the string is malformed, we'll treat it as 0.0.0 and emit a warning. This matches the approach from the previous code, to ignore incorrect version parts, ending up with a NUL-version. Signed-off-by: Stijn Devriendt <sde@unmatched.eu> --- core/artifacts_versions.c | 46 +++++++++++++++++++++++++++++++++++++++ core/swupdate.c | 2 ++ include/util.h | 1 + parser/parser.c | 3 +++ 4 files changed, 52 insertions(+)