| Message ID | 20230508162612.116091-1-sbabic@denx.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | copyhandler: allow zero sized files | expand |
Hi Stefano, > An empty file generates an error and SWUpdate stops the update. The > reason is that the size is detected and 0 was not assumed as valid size, > but it really is. > > Change logic and allows empty files to be copied. > > Signed-off-by: Stefano Babic <sbabic@denx.de> > --- > handlers/copy_handler.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c > index fa5f3c0..8ab4f01 100644 > --- a/handlers/copy_handler.c > +++ b/handlers/copy_handler.c > @@ -73,26 +73,31 @@ static int copy_single_file(const char *path, ssize_t size, struct img_type *img > /* > * try to detect the size if was not set in sw-description > */ > + bool valid_size = true; Hm, size is ssize_t, so it's signed. Hence, negative values may represent invalid sizes while >= 0 are valid sizes? > if (!size) { > - if (S_ISREG(statbuf.st_mode)) > + if (S_ISREG(statbuf.st_mode)) { > size = statbuf.st_size; > + } > else if (S_ISBLK(statbuf.st_mode) && (ioctl(fdin, BLKGETSIZE64, &size) < 0)) { > ERROR("Cannot get size of Block Device %s", path); > + valid_size = false; > } else if (S_ISCHR(statbuf.st_mode)) { > /* it is maybe a MTD device, just try */ > ret = ioctl(fdin, MEMGETINFO, &mtdinfo); > - if (!ret) > + if (!ret) { > size = mtdinfo.size; > + } else { > + valid_size = false; > + } > } > } > > - if (!size) { > + if (!valid_size) { > ERROR("Size cannot be detected for %s", path); > close(fdin); > return -ENODEV; > } > > - > if (pipe(pipes) < 0) { > ERROR("Could not create pipes for chained handler, existing..."); > close(fdin); > -- > 2.34.1 Kind regards, Christian
Hi Christian, On 09.05.23 10:28, 'Christian Storm' via swupdate wrote: > Hi Stefano, > >> An empty file generates an error and SWUpdate stops the update. The >> reason is that the size is detected and 0 was not assumed as valid size, >> but it really is. >> >> Change logic and allows empty files to be copied. >> >> Signed-off-by: Stefano Babic <sbabic@denx.de> >> --- >> handlers/copy_handler.c | 13 +++++++++---- >> 1 file changed, 9 insertions(+), 4 deletions(-) >> >> diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c >> index fa5f3c0..8ab4f01 100644 >> --- a/handlers/copy_handler.c >> +++ b/handlers/copy_handler.c >> @@ -73,26 +73,31 @@ static int copy_single_file(const char *path, ssize_t size, struct img_type *img >> /* >> * try to detect the size if was not set in sw-description >> */ >> + bool valid_size = true; > > Hm, size is ssize_t, so it's signed. Hence, negative values may > represent invalid sizes while >= 0 are valid sizes? Yes - I do not know if it is more readable, I put it in a V2 and repost, let's see. Best regards, Stefano > > >> if (!size) { >> - if (S_ISREG(statbuf.st_mode)) >> + if (S_ISREG(statbuf.st_mode)) { >> size = statbuf.st_size; >> + } >> else if (S_ISBLK(statbuf.st_mode) && (ioctl(fdin, BLKGETSIZE64, &size) < 0)) { >> ERROR("Cannot get size of Block Device %s", path); >> + valid_size = false; >> } else if (S_ISCHR(statbuf.st_mode)) { >> /* it is maybe a MTD device, just try */ >> ret = ioctl(fdin, MEMGETINFO, &mtdinfo); >> - if (!ret) >> + if (!ret) { >> size = mtdinfo.size; >> + } else { >> + valid_size = false; >> + } >> } >> } >> >> - if (!size) { >> + if (!valid_size) { >> ERROR("Size cannot be detected for %s", path); >> close(fdin); >> return -ENODEV; >> } >> >> - >> if (pipe(pipes) < 0) { >> ERROR("Could not create pipes for chained handler, existing..."); >> close(fdin); >> -- >> 2.34.1 > > > Kind regards, > Christian >
diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c index fa5f3c0..8ab4f01 100644 --- a/handlers/copy_handler.c +++ b/handlers/copy_handler.c @@ -73,26 +73,31 @@ static int copy_single_file(const char *path, ssize_t size, struct img_type *img /* * try to detect the size if was not set in sw-description */ + bool valid_size = true; if (!size) { - if (S_ISREG(statbuf.st_mode)) + if (S_ISREG(statbuf.st_mode)) { size = statbuf.st_size; + } else if (S_ISBLK(statbuf.st_mode) && (ioctl(fdin, BLKGETSIZE64, &size) < 0)) { ERROR("Cannot get size of Block Device %s", path); + valid_size = false; } else if (S_ISCHR(statbuf.st_mode)) { /* it is maybe a MTD device, just try */ ret = ioctl(fdin, MEMGETINFO, &mtdinfo); - if (!ret) + if (!ret) { size = mtdinfo.size; + } else { + valid_size = false; + } } } - if (!size) { + if (!valid_size) { ERROR("Size cannot be detected for %s", path); close(fdin); return -ENODEV; } - if (pipe(pipes) < 0) { ERROR("Could not create pipes for chained handler, existing..."); close(fdin);
An empty file generates an error and SWUpdate stops the update. The reason is that the size is detected and 0 was not assumed as valid size, but it really is. Change logic and allows empty files to be copied. Signed-off-by: Stefano Babic <sbabic@denx.de> --- handlers/copy_handler.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)