On Wed, May 24, 2023 at 08:18:32PM +0100, Richard W.M. Jones wrote: > On Wed, May 24, 2023 at 10:00:03AM -0500, Eric Blake wrote: > > diff --git a/copy/main.c b/copy/main.c > > index 391c0c4f..9449440e 100644 > > --- a/copy/main.c > > +++ b/copy/main.c > > @@ -371,7 +371,7 @@ main (int argc, char *argv[]) > > #else > > t = 1; > > #endif > > - threads = (unsigned)t; > > + threads = t; > > This one doesn't give any warning about truncating from long (64 bits) > to unsigned (32 bits)?
The compiler doesn't warn by default under -Wall. There are other knobs that can make it warn, but in this particular case, sysconf(_SC_PROCESSORS_ONLN) is unlikely to overflow (if you have a system with 4G cores, I'm jealous). Still, I don't mind tweaking to add an assert documenting our reasoning. > > > diff --git a/fuse/nbdfuse.c b/fuse/nbdfuse.c > > index 491f6db8..35d5ffac 100644 > > --- a/fuse/nbdfuse.c > > +++ b/fuse/nbdfuse.c > > @@ -415,7 +415,7 @@ main (int argc, char *argv[]) > > handles_append (&nbd, h); /* reserved above, so can't fail */ > > } > > } > > - connections = (unsigned)nbd.len; > > + connections = nbd.len; > > Similarly this would be size_t -> unsigned, also a truncation on 64 bit. Again, no compiler warning. And in this one, no assertion needed: nbd.len was just populated by a loop over the prior value of connections, and thus won't overflow. > > But if there are no warnings, then: > > Reviewed-by: Richard W.M. Jones <rjo...@redhat.com> > I decided to swap the order of these two patches (less churn); and touched up this one with the above-mentioned assert. Now committed as 96fdc62a..17d0bc6a -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
