Re: [Qemu-devel] Hacks for building on gcc 7 / Fedora 26

[Daniel P. Berrange]

On Thu, Jul 20, 2017 at 06:04:44PM +0100, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrange (address@hidden) wrote:
> > On Thu, Jul 20, 2017 at 05:15:49PM +0100, Dr. David Alan Gilbert wrote:
> > > * Daniel P. Berrange (address@hidden) wrote:
> > > > On Fri, Apr 07, 2017 at 03:38:47PM +0100, Dr. David Alan Gilbert wrote:
> > > > > Hi,
> > > > >    Fedora 26 has gcc 7.0.1 which has the normal compliment
> > > > > of new fussy warnings; so far I've posted :
> > > > > 
> > > > > tests/check-qdict: Fix missing brackets
> > > > > slirp/smb: Replace constant strings by glib string
> > > > > 
> > > > > that fix one actual mistake and work around something it's being
> > > > > fussy over.
> > > > > 
> > > > > But I've also got a pile of hacks, attached below that I'm
> > > > > not too sure what I'll do with them yet, but they're attached
> > > > > for anyone else trying to build.  Note they're smoke-only-tested.
> > > > > 
> > > > > I also have gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80346
> > > > > filed for what I reckon is a couple of overly pessimistic warnings.
> > > > 
> > > > 
> > > > > diff --git a/include/qemu/iov.h b/include/qemu/iov.h
> > > > > index bd9fd55b0a..ebb0221140 100644
> > > > > --- a/include/qemu/iov.h
> > > > > +++ b/include/qemu/iov.h
> > > > > @@ -46,7 +46,7 @@ static inline size_t
> > > > >  iov_from_buf(const struct iovec *iov, unsigned int iov_cnt,
> > > > >               size_t offset, const void *buf, size_t bytes)
> > > > >  {
> > > > > -    if (__builtin_constant_p(bytes) && iov_cnt &&
> > > > > +    if (__builtin_constant_p(bytes) && iov_cnt && bytes <= INT_MAX &&
> > > > >          offset <= iov[0].iov_len && bytes <= iov[0].iov_len - 
> > > > > offset) {
> > > > >          memcpy(iov[0].iov_base + offset, buf, bytes);
> > > > >          return bytes;
> > > > > @@ -59,7 +59,7 @@ static inline size_t
> > > > >  iov_to_buf(const struct iovec *iov, const unsigned int iov_cnt,
> > > > >             size_t offset, void *buf, size_t bytes)
> > > > >  {
> > > > > -    if (__builtin_constant_p(bytes) && iov_cnt &&
> > > > > +    if (__builtin_constant_p(bytes) && iov_cnt && bytes <= INT_MAX &&
> > > > >          offset <= iov[0].iov_len && bytes <= iov[0].iov_len - 
> > > > > offset) {
> > > > >          memcpy(buf, iov[0].iov_base + offset, bytes);
> > > > >          return bytes;
> > > 
> > > tbh I don't know what the right fix for this is;  the gcc discussion
> > > confused me as to why it thinks it can be a valid case.
> > 
> > Even if gcc is broken in issuing a warning here, we still need to
> > make it quiet so people on F26 and similarly new distros can build
> > without warnings.
> 
> I agree.
> 
> > IMHO your patch is ok, or we could be alittle more explicit about
> > catching just the case where you pass -1 for bytes, and have
> > 
> >   && bytes != -1
> 
> This seems bizarre to me since bytes is   size_t bytes   and size_t
> is unsigned, so I'd have sympathy for a compiler that warned that
> bytes != -1 was always true.

Could be paranoid and do    "&& bytes !=  (size_t)-1"

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|