| Message ID | 20180116150902.20137-1-christian.storm@siemens.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Fix Wsign-compare for int with ARRAY_SIZE() comparison | expand |
Hi Christian, On 16/01/2018 16:09, Christian Storm wrote: > include/util.h's ARRAY_SIZE() uses a sizeof() calculation > which returns unsigned int. Hence fix Wsign-compare by > making the comparison variable unsigned int. > > Signed-off-by: Christian Storm <christian.storm@siemens.com> > --- > corelib/installer.c | 6 ++---- > corelib/stream_interface.c | 4 +--- > 2 files changed, 3 insertions(+), 7 deletions(-) > > diff --git a/corelib/installer.c b/corelib/installer.c > index 4f58794..592923d 100644 > --- a/corelib/installer.c > +++ b/corelib/installer.c > @@ -423,11 +423,10 @@ static void remove_sw_file(char __attribute__ ((__unused__)) *fname) > static void cleaup_img_entry(struct img_type *img) > { > char *fn; > - int i; > const char *tmp[] = { get_tmpdirscripts(), get_tmpdir() }; > > if (img->fname[0]) { > - for (i = 0; i < ARRAY_SIZE(tmp); i++) { > + for (unsigned int i = 0; i < ARRAY_SIZE(tmp); i++) { > if (asprintf(&fn, "%s%s", tmp[i], img->fname) == ENOMEM_ASPRINTF) { > ERROR("Path too long: %s%s", tmp[i], img->fname); > } else { > @@ -444,7 +443,6 @@ void cleanup_files(struct swupdate_cfg *software) { > struct img_type *img; > struct hw_type *hw; > const char* TMPDIR = get_tmpdir(); > - int count; > struct imglist *list[] = {&software->scripts, &software->bootscripts}; > > LIST_FOREACH(img, &software->images, next) { > @@ -459,7 +457,7 @@ void cleanup_files(struct swupdate_cfg *software) { > free(img); > } > > - for (count = 0; count < ARRAY_SIZE(list); count++) { > + for (unsigned int count = 0; count < ARRAY_SIZE(list); count++) { Right, thanks for fixing this. > LIST_FOREACH(img, list[count], next) { > cleaup_img_entry(img); > > diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c > index 98b8ca9..a5fd4a1 100644 > --- a/corelib/stream_interface.c > +++ b/corelib/stream_interface.c > @@ -173,13 +173,11 @@ static int extract_files(int fd, struct swupdate_cfg *software) > break; > } > > - int i; > - > struct imglist *list[] = {&software->images, > &software->scripts, > &software->bootscripts}; > > - for (i = 0; i < ARRAY_SIZE(list); i++) { > + for (unsigned int i = 0; i < ARRAY_SIZE(list); i++) { > skip = check_if_required(list[i], &fdh, > (list[i] == &software->images) ? > &software->installed_sw_list : NULL, > Reviewed-by: Stefano Babic <sbabic@denx.de> Best regards, Stefano Babic
Hi Christian and Stefano, Am 16.01.2018 um 16:44 schrieb Stefano Babic: > On 16/01/2018 16:09, Christian Storm wrote: >> include/util.h's ARRAY_SIZE() uses a sizeof() calculation >> which returns unsigned int. Hence fix Wsign-compare by >> making the comparison variable unsigned int. >> >> Signed-off-by: Christian Storm <christian.storm@siemens.com> >> --- >> corelib/installer.c | 6 ++---- >> corelib/stream_interface.c | 4 +--- >> 2 files changed, 3 insertions(+), 7 deletions(-) >> >> diff --git a/corelib/installer.c b/corelib/installer.c >> index 4f58794..592923d 100644 >> --- a/corelib/installer.c >> +++ b/corelib/installer.c >> @@ -423,11 +423,10 @@ static void remove_sw_file(char __attribute__ ((__unused__)) *fname) >> static void cleaup_img_entry(struct img_type *img) >> { >> char *fn; >> - int i; >> const char *tmp[] = { get_tmpdirscripts(), get_tmpdir() }; >> >> if (img->fname[0]) { >> - for (i = 0; i < ARRAY_SIZE(tmp); i++) { >> + for (unsigned int i = 0; i < ARRAY_SIZE(tmp); i++) { >> if (asprintf(&fn, "%s%s", tmp[i], img->fname) == ENOMEM_ASPRINTF) { >> ERROR("Path too long: %s%s", tmp[i], img->fname); >> } else { >> @@ -444,7 +443,6 @@ void cleanup_files(struct swupdate_cfg *software) { >> struct img_type *img; >> struct hw_type *hw; >> const char* TMPDIR = get_tmpdir(); >> - int count; >> struct imglist *list[] = {&software->scripts, &software->bootscripts}; >> >> LIST_FOREACH(img, &software->images, next) { >> @@ -459,7 +457,7 @@ void cleanup_files(struct swupdate_cfg *software) { >> free(img); >> } >> >> - for (count = 0; count < ARRAY_SIZE(list); count++) { >> + for (unsigned int count = 0; count < ARRAY_SIZE(list); count++) { > Right, thanks for fixing this. > >> LIST_FOREACH(img, list[count], next) { >> cleaup_img_entry(img); >> >> diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c >> index 98b8ca9..a5fd4a1 100644 >> --- a/corelib/stream_interface.c >> +++ b/corelib/stream_interface.c >> @@ -173,13 +173,11 @@ static int extract_files(int fd, struct swupdate_cfg *software) >> break; >> } >> >> - int i; >> - >> struct imglist *list[] = {&software->images, >> &software->scripts, >> &software->bootscripts}; >> >> - for (i = 0; i < ARRAY_SIZE(list); i++) { >> + for (unsigned int i = 0; i < ARRAY_SIZE(list); i++) { >> skip = check_if_required(list[i], &fdh, >> (list[i] == &software->images) ? >> &software->installed_sw_list : NULL, >> > Reviewed-by: Stefano Babic <sbabic@denx.de> This patch requires C99 standard support. Maybe we should set the c standard explicit or document it. Best regards Stefan
Hi Stefan, On 16/01/2018 20:01, Stefan Herbrechtsmeier wrote: > Hi Christian and Stefano, > > Am 16.01.2018 um 16:44 schrieb Stefano Babic: >> On 16/01/2018 16:09, Christian Storm wrote: >>> include/util.h's ARRAY_SIZE() uses a sizeof() calculation >>> which returns unsigned int. Hence fix Wsign-compare by >>> making the comparison variable unsigned int. >>> >>> Signed-off-by: Christian Storm <christian.storm@siemens.com> >>> --- >>> corelib/installer.c | 6 ++---- >>> corelib/stream_interface.c | 4 +--- >>> 2 files changed, 3 insertions(+), 7 deletions(-) >>> >>> diff --git a/corelib/installer.c b/corelib/installer.c >>> index 4f58794..592923d 100644 >>> --- a/corelib/installer.c >>> +++ b/corelib/installer.c >>> @@ -423,11 +423,10 @@ static void remove_sw_file(char __attribute__ >>> ((__unused__)) *fname) >>> static void cleaup_img_entry(struct img_type *img) >>> { >>> char *fn; >>> - int i; >>> const char *tmp[] = { get_tmpdirscripts(), get_tmpdir() }; >>> if (img->fname[0]) { >>> - for (i = 0; i < ARRAY_SIZE(tmp); i++) { >>> + for (unsigned int i = 0; i < ARRAY_SIZE(tmp); i++) { >>> if (asprintf(&fn, "%s%s", tmp[i], img->fname) == >>> ENOMEM_ASPRINTF) { >>> ERROR("Path too long: %s%s", tmp[i], img->fname); >>> } else { >>> @@ -444,7 +443,6 @@ void cleanup_files(struct swupdate_cfg *software) { >>> struct img_type *img; >>> struct hw_type *hw; >>> const char* TMPDIR = get_tmpdir(); >>> - int count; >>> struct imglist *list[] = {&software->scripts, >>> &software->bootscripts}; >>> LIST_FOREACH(img, &software->images, next) { >>> @@ -459,7 +457,7 @@ void cleanup_files(struct swupdate_cfg *software) { >>> free(img); >>> } >>> - for (count = 0; count < ARRAY_SIZE(list); count++) { >>> + for (unsigned int count = 0; count < ARRAY_SIZE(list); count++) { >> Right, thanks for fixing this. >> >>> LIST_FOREACH(img, list[count], next) { >>> cleaup_img_entry(img); >>> diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c >>> index 98b8ca9..a5fd4a1 100644 >>> --- a/corelib/stream_interface.c >>> +++ b/corelib/stream_interface.c >>> @@ -173,13 +173,11 @@ static int extract_files(int fd, struct >>> swupdate_cfg *software) >>> break; >>> } >>> - int i; >>> - >>> struct imglist *list[] = {&software->images, >>> &software->scripts, >>> &software->bootscripts}; >>> - for (i = 0; i < ARRAY_SIZE(list); i++) { >>> + for (unsigned int i = 0; i < ARRAY_SIZE(list); i++) { >>> skip = check_if_required(list[i], &fdh, >>> (list[i] == &software->images) ? >>> &software->installed_sw_list : NULL, >>> >> Reviewed-by: Stefano Babic <sbabic@denx.de> > > This patch requires C99 standard support. Well, not only this patch..... > Maybe we should set the c > standard explicit or document it. Something about compiler requirement is missing in documentation. I vote to add this to documentation. Best regards, Stefano -- ===================================================================== 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: sbabic@denx.de =====================================================================
Hi, > > This patch requires C99 standard support. > > Well, not only this patch..... > > > Maybe we should set the c > > standard explicit or document it. Well, it's actually set, explicitly. See Makefile.flags:10: KBUILD_CPPFLAGS += $(call cc-option,-std=gnu99,) OK, the gnu "dialect" but nonetheless C99. > Something about compiler requirement is missing in documentation. > I vote to add this to documentation. Hm, what compiler nowadays doesn't support that standard coming from the last millennium, nearly twenty years after its invention? :) That said, I'm not against documenting this at, e.g., source/swupdate.rst:201 Kind regards, Christian
On 17/01/2018 08:56, Christian Storm wrote: > Hi, > >>> This patch requires C99 standard support. >> >> Well, not only this patch..... >> >>> Maybe we should set the c >>> standard explicit or document it. > > Well, it's actually set, explicitly. See Makefile.flags:10: > KBUILD_CPPFLAGS += $(call cc-option,-std=gnu99,) > OK, the gnu "dialect" but nonetheless C99. > >> Something about compiler requirement is missing in documentation. >> I vote to add this to documentation. > > Hm, what compiler nowadays doesn't support that standard coming > from the last millennium, nearly twenty years after its invention? :) Indeed. I thought when I started the project if I had to stick with plain K&R language, maybe if SWUpdate can be ported to a lower (=microcontroller) device. But all compilers support nowadays the new standard. > That said, I'm not against documenting this at, e.g., > source/swupdate.rst:201 > Best regards, Stefano
diff --git a/corelib/installer.c b/corelib/installer.c index 4f58794..592923d 100644 --- a/corelib/installer.c +++ b/corelib/installer.c @@ -423,11 +423,10 @@ static void remove_sw_file(char __attribute__ ((__unused__)) *fname) static void cleaup_img_entry(struct img_type *img) { char *fn; - int i; const char *tmp[] = { get_tmpdirscripts(), get_tmpdir() }; if (img->fname[0]) { - for (i = 0; i < ARRAY_SIZE(tmp); i++) { + for (unsigned int i = 0; i < ARRAY_SIZE(tmp); i++) { if (asprintf(&fn, "%s%s", tmp[i], img->fname) == ENOMEM_ASPRINTF) { ERROR("Path too long: %s%s", tmp[i], img->fname); } else { @@ -444,7 +443,6 @@ void cleanup_files(struct swupdate_cfg *software) { struct img_type *img; struct hw_type *hw; const char* TMPDIR = get_tmpdir(); - int count; struct imglist *list[] = {&software->scripts, &software->bootscripts}; LIST_FOREACH(img, &software->images, next) { @@ -459,7 +457,7 @@ void cleanup_files(struct swupdate_cfg *software) { free(img); } - for (count = 0; count < ARRAY_SIZE(list); count++) { + for (unsigned int count = 0; count < ARRAY_SIZE(list); count++) { LIST_FOREACH(img, list[count], next) { cleaup_img_entry(img); diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c index 98b8ca9..a5fd4a1 100644 --- a/corelib/stream_interface.c +++ b/corelib/stream_interface.c @@ -173,13 +173,11 @@ static int extract_files(int fd, struct swupdate_cfg *software) break; } - int i; - struct imglist *list[] = {&software->images, &software->scripts, &software->bootscripts}; - for (i = 0; i < ARRAY_SIZE(list); i++) { + for (unsigned int i = 0; i < ARRAY_SIZE(list); i++) { skip = check_if_required(list[i], &fdh, (list[i] == &software->images) ? &software->installed_sw_list : NULL,
include/util.h's ARRAY_SIZE() uses a sizeof() calculation which returns unsigned int. Hence fix Wsign-compare by making the comparison variable unsigned int. Signed-off-by: Christian Storm <christian.storm@siemens.com> --- corelib/installer.c | 6 ++---- corelib/stream_interface.c | 4 +--- 2 files changed, 3 insertions(+), 7 deletions(-)