| Message ID | 20181101064937.28731-1-peng.fan@nxp.com |
|---|---|
| State | Superseded |
| Delegated to: | Stefano Babic |
| Headers | show |
| Series | [U-Boot,1/4] tools: imx8image: check lseek return value | expand |
Hi Peng, On Thu, Nov 1, 2018 at 3:42 AM Peng Fan <peng.fan@nxp.com> wrote: > size = sbuf.st_size; > - lseek(ifd, offset, SEEK_SET); > + if (lseek(ifd, offset, SEEK_SET) == -1) { > + fprintf(stderr, "%s: lseek error %s\n", > + __func__, strerror(errno)); > + exit(EXIT_FAILURE); > + } What about something like this instead? ret = lseek(ifd, offset, SEEK_SET); if (ret < 0) { fprintf(stderr, "%s: lseek error: %d\n", __func__, ret); exit(EXIT_FAILURE); }
Hi Fabio, > -----Original Message----- > From: Fabio Estevam [mailto:festevam@gmail.com] > Sent: 2018年11月1日 18:18 > To: Peng Fan <peng.fan@nxp.com> > Cc: Stefano Babic <sbabic@denx.de>; Fabio Estevam > <fabio.estevam@nxp.com>; U-Boot-Denx <u-boot@lists.denx.de>; dl-linux-imx > <linux-imx@nxp.com> > Subject: Re: [U-Boot] [PATCH 1/4] tools: imx8image: check lseek return value > Hi Peng, > > On Thu, Nov 1, 2018 at 3:42 AM Peng Fan <peng.fan@nxp.com> wrote: > > > size = sbuf.st_size; > > - lseek(ifd, offset, SEEK_SET); > > + if (lseek(ifd, offset, SEEK_SET) == -1) { > > + fprintf(stderr, "%s: lseek error %s\n", > > + __func__, strerror(errno)); > > + exit(EXIT_FAILURE); > > + } > > What about something like this instead? > > ret = lseek(ifd, offset, SEEK_SET); > if (ret < 0) { > fprintf(stderr, "%s: lseek error: %d\n", __func__, ret); > exit(EXIT_FAILURE); > } lseek only returns -1 when error. Is the coding style you proposed is preferred than what this patch has? I saw fit_image use if (xxx < 0) and fw_env.c use if (xxx == -1), there are also others use ret = lseek(). Thanks, Peng.
Hi Peng, On Thu, Nov 1, 2018 at 9:15 AM Peng Fan <peng.fan@nxp.com> wrote: > lseek only returns -1 when error. Is the coding style you proposed is preferred than > what this patch has? In case lseek changes someday to return something else, checking for (ret < 0) will still work, so it is a more robust error handling.
Hi Fabio, > -----Original Message----- > From: Fabio Estevam [mailto:festevam@gmail.com] > Sent: 2018年11月1日 20:18 > To: Peng Fan <peng.fan@nxp.com> > Cc: Stefano Babic <sbabic@denx.de>; Fabio Estevam > <fabio.estevam@nxp.com>; U-Boot-Denx <u-boot@lists.denx.de>; dl-linux-imx > <linux-imx@nxp.com> > Subject: Re: [U-Boot] [PATCH 1/4] tools: imx8image: check lseek return value > > Hi Peng, > > On Thu, Nov 1, 2018 at 9:15 AM Peng Fan <peng.fan@nxp.com> wrote: > > > lseek only returns -1 when error. Is the coding style you proposed is > > preferred than what this patch has? > > In case lseek changes someday to return something else, checking for (ret < 0) > will still work, so it is a more robust error handling. lseek prototype, return value, in the spec http://pubs.opengroup.org/onlinepubs/9699919799/ Understand (ret < 0) is a good practice, but I do not think lseek return value will change. Thanks, Peng.
diff --git a/tools/imx8image.c b/tools/imx8image.c index e6b0a146b6..eb039b583d 100644 --- a/tools/imx8image.c +++ b/tools/imx8image.c @@ -333,7 +333,12 @@ static void copy_file_aligned(int ifd, const char *datafile, int offset, } size = sbuf.st_size; - lseek(ifd, offset, SEEK_SET); + if (lseek(ifd, offset, SEEK_SET) == -1) { + fprintf(stderr, "%s: lseek error %s\n", + __func__, strerror(errno)); + exit(EXIT_FAILURE); + } + if (write(ifd, ptr, size) != size) { fprintf(stderr, "Write error %s\n", strerror(errno)); exit(EXIT_FAILURE); @@ -387,7 +392,12 @@ static void copy_file (int ifd, const char *datafile, int pad, int offset) } size = sbuf.st_size; - lseek(ifd, offset, SEEK_SET); + if (lseek(ifd, offset, SEEK_SET) == -1) { + fprintf(stderr, "%s: lseek error %s\n", + __func__, strerror(errno)); + exit(EXIT_FAILURE); + } + if (write(ifd, ptr, size) != size) { fprintf(stderr, "Write error %s\n", strerror(errno)); @@ -883,7 +893,11 @@ static int build_container(soc_type_t soc, uint32_t sector_size, } while (img_sp->option != NO_IMG); /* Add padding or skip appended container */ - lseek(ofd, file_padding, SEEK_SET); + if (lseek(ofd, file_padding, SEEK_SET) == -1) { + fprintf(stderr, "%s: lseek error %s\n", + __func__, strerror(errno)); + exit(EXIT_FAILURE); + } /* Note: Image offset are not contained in the image */ tmp = flatten_container_header(&imx_header, container + 1, &size,
Check lseek return value. Fix Coverity CID: 184236 184235 184232 Signed-off-by: Peng Fan <peng.fan@nxp.com> --- tools/imx8image.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)