| Message ID | 20250516143048.2962050-1-joakim.tjernlund@infinera.com |
|---|---|
| State | Rejected |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | COPY handler: rm FTW_CONTINUE and FTW_STOP | expand |
On 5/16/25 16:30, 'Joakim Tjernlund' via swupdate wrote: > These are a GNU extension not preset in MUSL. FTW_ACTIONRETVAL flag > must be passed to use it. As this flag is missing we can replace > FTW_CONTINUE and FTW_STOP with 0 resp. 1 and maintain same function. > ...or add defines into include.h (or even here) in case they are not defined.... > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > --- > handlers/copy_handler.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c > index 222bc9f5..bea0899e 100644 > --- a/handlers/copy_handler.c > +++ b/handlers/copy_handler.c > @@ -151,7 +151,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > char *dst; > struct stat statbufdst; > struct img_type cpyimg; > - int ret, result = FTW_CONTINUE; > + int ret, result = 0; > > if (strstr(fpath, copyfrom) != fpath) > return result; > @@ -159,7 +159,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > relpath = fpath + strlen(copyfrom); > > if (!strlen(relpath)) > - return FTW_CONTINUE; > + return 0; > > dst = malloc(strlen(base_img->path) + strlen(relpath) + 1); > strcpy(dst, base_img->path); > @@ -189,7 +189,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > */ > swupdate_progress_addstep(); > if (copy_single_file(fpath, 0, &cpyimg, chained_handler)) > - result = FTW_STOP; > + result = 1; > } > > free(dst);
On Fri, 2025-05-16 at 16:34 +0200, Stefano Babic wrote: > On 5/16/25 16:30, 'Joakim Tjernlund' via swupdate wrote: > > These are a GNU extension not preset in MUSL. FTW_ACTIONRETVAL flag > > must be passed to use it. As this flag is missing we can replace > > FTW_CONTINUE and FTW_STOP with 0 resp. 1 and maintain same function. > > > > ...or add defines into include.h (or even here) in case they are not > defined.... But here they are wrongly used even for GNU, you can only use these if you set FTW_ACTIONRETVAL Making #defines will hide errors if someone tries adds FTW_ACTIONRETVAL later Jocke > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > --- > > handlers/copy_handler.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c > > index 222bc9f5..bea0899e 100644 > > --- a/handlers/copy_handler.c > > +++ b/handlers/copy_handler.c > > @@ -151,7 +151,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > > char *dst; > > struct stat statbufdst; > > struct img_type cpyimg; > > - int ret, result = FTW_CONTINUE; > > + int ret, result = 0; > > > > if (strstr(fpath, copyfrom) != fpath) > > return result; > > @@ -159,7 +159,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > > relpath = fpath + strlen(copyfrom); > > > > if (!strlen(relpath)) > > - return FTW_CONTINUE; > > + return 0; > > > > dst = malloc(strlen(base_img->path) + strlen(relpath) + 1); > > strcpy(dst, base_img->path); > > @@ -189,7 +189,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > > */ > > swupdate_progress_addstep(); > > if (copy_single_file(fpath, 0, &cpyimg, chained_handler)) > > - result = FTW_STOP; > > + result = 1; > > } > > > > free(dst);
Hi Jocke, On 16.05.25 18:17, 'Joakim Tjernlund (Nokia)' via swupdate wrote: > On Fri, 2025-05-16 at 16:34 +0200, Stefano Babic wrote: >> On 5/16/25 16:30, 'Joakim Tjernlund' via swupdate wrote: >>> These are a GNU extension not preset in MUSL. FTW_ACTIONRETVAL flag >>> must be passed to use it. As this flag is missing we can replace >>> FTW_CONTINUE and FTW_STOP with 0 resp. 1 and maintain same function. >>> >> >> ...or add defines into include.h (or even here) in case they are not >> defined.... > > But here they are wrongly used even for GNU, you can only use these if you set FTW_ACTIONRETVAL > Making #defines will hide errors if someone tries adds FTW_ACTIONRETVAL later I am yet not understanding where is the issue. Commit 58a1ec0708 addresses exactly this topic, and set FTW_CONTINUE and FTW_STOP in case they are not set, that is for MUSL ( as the commit message says). Regards, Stefano > > Jocke >> >>> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> >>> --- >>> handlers/copy_handler.c | 6 +++--- >>> 1 file changed, 3 insertions(+), 3 deletions(-) >>> >>> diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c >>> index 222bc9f5..bea0899e 100644 >>> --- a/handlers/copy_handler.c >>> +++ b/handlers/copy_handler.c >>> @@ -151,7 +151,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, >>> char *dst; >>> struct stat statbufdst; >>> struct img_type cpyimg; >>> - int ret, result = FTW_CONTINUE; >>> + int ret, result = 0; >>> >>> if (strstr(fpath, copyfrom) != fpath) >>> return result; >>> @@ -159,7 +159,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, >>> relpath = fpath + strlen(copyfrom); >>> >>> if (!strlen(relpath)) >>> - return FTW_CONTINUE; >>> + return 0; >>> >>> dst = malloc(strlen(base_img->path) + strlen(relpath) + 1); >>> strcpy(dst, base_img->path); >>> @@ -189,7 +189,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, >>> */ >>> swupdate_progress_addstep(); >>> if (copy_single_file(fpath, 0, &cpyimg, chained_handler)) >>> - result = FTW_STOP; >>> + result = 1; >>> } >>> >>> free(dst); >
On Fri, 2025-05-16 at 23:45 +0200, Stefano Babic wrote: > > Hi Jocke, Hi again :) > > On 16.05.25 18:17, 'Joakim Tjernlund (Nokia)' via swupdate wrote: > > On Fri, 2025-05-16 at 16:34 +0200, Stefano Babic wrote: > > > On 5/16/25 16:30, 'Joakim Tjernlund' via swupdate wrote: > > > > These are a GNU extension not preset in MUSL. FTW_ACTIONRETVAL flag > > > > must be passed to use it. As this flag is missing we can replace > > > > FTW_CONTINUE and FTW_STOP with 0 resp. 1 and maintain same function. > > > > > > > > > > ...or add defines into include.h (or even here) in case they are not > > > defined.... > > > > But here they are wrongly used even for GNU, you can only use these if you set FTW_ACTIONRETVAL > > Making #defines will hide errors if someone tries adds FTW_ACTIONRETVAL later > > I am yet not understanding where is the issue. Commit 58a1ec0708 > addresses exactly this topic, and set FTW_CONTINUE and FTW_STOP in case > they are not set, that is for MUSL ( as the commit message says). Ahh, I was working of the 2024 release as that is what we currently use and missed this :( My point with just #defining FTW_CONTINUE/FTW_STOP is that it gives you the impression that a developer could start using FTW_ACTIONRETVAL and that would break on non glibc based systems. Anyhow you have a fix already so you can drop this one. Regards Joakim > Regards, > Stefano > > > > > Jocke > > > > > > > Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> > > > > --- > > > > handlers/copy_handler.c | 6 +++--- > > > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c > > > > index 222bc9f5..bea0899e 100644 > > > > --- a/handlers/copy_handler.c > > > > +++ b/handlers/copy_handler.c > > > > @@ -151,7 +151,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > > > > char *dst; > > > > struct stat statbufdst; > > > > struct img_type cpyimg; > > > > - int ret, result = FTW_CONTINUE; > > > > + int ret, result = 0; > > > > > > > > if (strstr(fpath, copyfrom) != fpath) > > > > return result; > > > > @@ -159,7 +159,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > > > > relpath = fpath + strlen(copyfrom); > > > > > > > > if (!strlen(relpath)) > > > > - return FTW_CONTINUE; > > > > + return 0; > > > > > > > > dst = malloc(strlen(base_img->path) + strlen(relpath) + 1); > > > > strcpy(dst, base_img->path); > > > > @@ -189,7 +189,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, > > > > */ > > > > swupdate_progress_addstep(); > > > > if (copy_single_file(fpath, 0, &cpyimg, chained_handler)) > > > > - result = FTW_STOP; > > > > + result = 1; > > > > } > > > > > > > > free(dst); > >
diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c index 222bc9f5..bea0899e 100644 --- a/handlers/copy_handler.c +++ b/handlers/copy_handler.c @@ -151,7 +151,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, char *dst; struct stat statbufdst; struct img_type cpyimg; - int ret, result = FTW_CONTINUE; + int ret, result = 0; if (strstr(fpath, copyfrom) != fpath) return result; @@ -159,7 +159,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, relpath = fpath + strlen(copyfrom); if (!strlen(relpath)) - return FTW_CONTINUE; + return 0; dst = malloc(strlen(base_img->path) + strlen(relpath) + 1); strcpy(dst, base_img->path); @@ -189,7 +189,7 @@ static int recurse_directory(const char *fpath, const struct stat *sb, */ swupdate_progress_addstep(); if (copy_single_file(fpath, 0, &cpyimg, chained_handler)) - result = FTW_STOP; + result = 1; } free(dst);
These are a GNU extension not preset in MUSL. FTW_ACTIONRETVAL flag must be passed to use it. As this flag is missing we can replace FTW_CONTINUE and FTW_STOP with 0 resp. 1 and maintain same function. Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com> --- handlers/copy_handler.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)