On Sat, Jan 12, 2019 at 3:18 AM Sam Ravnborg <sam@xxxxxxxxxxxx> wrote:
>
> Hi Andrey.
>
> > +static int digest_update_from_fd(struct digest *d, int fd,
> > + ulong start, ulong size)
> > +{
> > + unsigned char *buf = xmalloc(PAGE_SIZE);
> > + int ret = 0;
> > +
> > + ret = lseek(fd, start, SEEK_SET);
> > + if (ret == -1) {
> > + perror("lseek");
> > + goto out_free;
> > + }
> lseek return (off_t)-1 - should ret be of type off_t to make this correct?
>
I tried to avoid changing original code too much in this patch, just
to keep the scope of this patch to just refactoring. But yeah, I think
there's definitely a problem that a larger value of "start" passed to
lseek() will result in a failure due to "loff_t" downcast to "int".
Another problem with this is that lseek() doesn't really return a
detailed error code, so what this function should really do is capture
it from "errno" and this way "ret" can be kept as "int".
There's a couple of more places in digest.c that don't really capture
"errno", so I think I'll create a separate follow up series to fix
that, instead of re-spinning this one.
Thanks,
Andrey Smirnov
> The code looks more readable with the two helper functions.
> So +1 from me.
>
> Sam
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
