Re: [PATCH] usb: Fix clang build
On Wed, Jan 20, 2021 at 12:20:43AM +0100, Philippe Mathieu-Daudé wrote: > Hi Eric, > > On 1/20/21 12:07 AM, Eric Blake wrote: > > ../hw/usb/dev-uas.c:157:31: error: field 'status' with variable sized type > > 'uas_iu' not at the end of a struct or class is a GNU extension > > [-Werror,-Wgnu-variable-sized-type-not-at-end] > > uas_iustatus; > > ^ > > > > Fix this by specifying a size for the add_cdb member; and at present, > > the code does not actually use that field other than for the size > > chosen for the packed uas_iu_command struct, and the choice of one > > byte does not change the size of the uas_iu union. > > I sent a maybe safer approach (from the bus PoV): > https://www.mail-archive.com/qemu-block@nongnu.org/msg79192.html > > Do you mind reviewing it? > > > > > Signed-off-by: Eric Blake > > --- > > > > I'm not sure why none of our CI tools pick up this particular clang > > build failure; I hit it on Fedora 33 when configuring to build the > > entire tree with clang. > > Same issue after upgrading to f33. I sent a patch to bump our CI: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg774117.html > > To track Fedora releases I was thinking about a gitlab job checking > if we are using the latest, else failing; smth as: > > $ curl https://getfedora.org/ | grep -q 'Fedora 33 released' We'd be better off just adding a job that targets Fedora rawhide IMHO so we get immediate notice that a toolchain update is going to hurt us, instead of waiting until the next Fedora is already released to find the problem. 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 :|
Re: [PATCH] usb: Fix clang build
On 1/20/21 12:20 AM, Philippe Mathieu-Daudé wrote: > Hi Eric, > > On 1/20/21 12:07 AM, Eric Blake wrote: >> ../hw/usb/dev-uas.c:157:31: error: field 'status' with variable sized type >> 'uas_iu' not at the end of a struct or class is a GNU extension >> [-Werror,-Wgnu-variable-sized-type-not-at-end] >> uas_iustatus; >> ^ >> >> Fix this by specifying a size for the add_cdb member; and at present, >> the code does not actually use that field other than for the size >> chosen for the packed uas_iu_command struct, and the choice of one >> byte does not change the size of the uas_iu union. > > I sent a maybe safer approach (from the bus PoV): > https://www.mail-archive.com/qemu-block@nongnu.org/msg79192.html > > Do you mind reviewing it? > >> >> Signed-off-by: Eric Blake >> --- >> >> I'm not sure why none of our CI tools pick up this particular clang >> build failure; I hit it on Fedora 33 when configuring to build the >> entire tree with clang. BTW first report is from 28 Sep 2020 (Ed): https://www.mail-archive.com/qemu-devel@nongnu.org/msg745525.html Then on 23 Oct 2020 (Daniele): https://www.mail-archive.com/qemu-devel@nongnu.org/msg753674.html Then on 10 Nov 2020 (Han): https://www.mail-archive.com/qemu-devel@nongnu.org/msg759108.html > Same issue after upgrading to f33. I sent a patch to bump our CI: > https://www.mail-archive.com/qemu-devel@nongnu.org/msg774117.html > > To track Fedora releases I was thinking about a gitlab job checking > if we are using the latest, else failing; smth as: > > $ curl https://getfedora.org/ | grep -q 'Fedora 33 released' >
Re: [PATCH] usb: Fix clang build
Hi Eric, On 1/20/21 12:07 AM, Eric Blake wrote: > ../hw/usb/dev-uas.c:157:31: error: field 'status' with variable sized type > 'uas_iu' not at the end of a struct or class is a GNU extension > [-Werror,-Wgnu-variable-sized-type-not-at-end] > uas_iustatus; > ^ > > Fix this by specifying a size for the add_cdb member; and at present, > the code does not actually use that field other than for the size > chosen for the packed uas_iu_command struct, and the choice of one > byte does not change the size of the uas_iu union. I sent a maybe safer approach (from the bus PoV): https://www.mail-archive.com/qemu-block@nongnu.org/msg79192.html Do you mind reviewing it? > > Signed-off-by: Eric Blake > --- > > I'm not sure why none of our CI tools pick up this particular clang > build failure; I hit it on Fedora 33 when configuring to build the > entire tree with clang. Same issue after upgrading to f33. I sent a patch to bump our CI: https://www.mail-archive.com/qemu-devel@nongnu.org/msg774117.html To track Fedora releases I was thinking about a gitlab job checking if we are using the latest, else failing; smth as: $ curl https://getfedora.org/ | grep -q 'Fedora 33 released'
[PATCH] usb: Fix clang build
../hw/usb/dev-uas.c:157:31: error: field 'status' with variable sized type
'uas_iu' not at the end of a struct or class is a GNU extension
[-Werror,-Wgnu-variable-sized-type-not-at-end]
uas_iustatus;
^
Fix this by specifying a size for the add_cdb member; and at present,
the code does not actually use that field other than for the size
chosen for the packed uas_iu_command struct, and the choice of one
byte does not change the size of the uas_iu union.
Signed-off-by: Eric Blake
---
I'm not sure why none of our CI tools pick up this particular clang
build failure; I hit it on Fedora 33 when configuring to build the
entire tree with clang.
hw/usb/dev-uas.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/usb/dev-uas.c b/hw/usb/dev-uas.c
index cec071d96c49..904d6ffa2938 100644
--- a/hw/usb/dev-uas.c
+++ b/hw/usb/dev-uas.c
@@ -70,7 +70,7 @@ typedef struct {
uint8_treserved_2;
uint64_t lun;
uint8_tcdb[16];
-uint8_tadd_cdb[];
+uint8_tadd_cdb[1];
} QEMU_PACKED uas_iu_command;
typedef struct {
--
2.30.0
