Re: [libvirt] [PATCHv4 10/11] Introduce QEMU_CAPS_VIRTIO_PCI_DISABLE_LEGACY

On 08/16/2016 07:50 AM, Andrea Bolognani wrote: > On Thu, 2016-08-11 at 13:57 +0200, Ján Tomko wrote: >> +static struct virQEMUCapsPropObjects virQEMUCapsPropObjects[] = { > Please, don't :) > > Use something like virQEMUCapsPropTypeObjects (to mirror the > existing virQEMUCapsObjectTypeProps), or > virQEMUCapsPropObjectsType, or anything really - just make sure > the name of the type and the name of the variable containing a > bunch of instances of said type are not the same. > >> static void >> +virQEMUCapsProcessProps(virQEMUCapsPtr qemuCaps, >> + size_t nprops, >> + struct virQEMUCapsPropObjects *props, >> + const char *object, >> + size_t nvalues, >> + char *const*values) >> +{ >> + size_t i, j; >> + >> + for (i = 0; i < nprops; i++) { >> + if (virQEMUCapsGet(qemuCaps, props[i].flag)) >> + continue; >> + >> + for (j = 0; j < nvalues; j++) { >> + if (STREQ(values[j], props[i].prop)) { >> + if (virStringArrayHasString((char **)props[i].objects, object)) > Rather than casting a const char ** to char **, which happens > in other places as well, it would be IMHO much better to make > virStringArrayHasString() accept a const char ** as the first > argument. > > And guess what? I just posted a patch[1] that does exactly > that :) > > > Everything else looks good. I'll ACK this pending the two changes abologna suggested. If you're confident you won't want to change this, but might be delayed in redoing the rest of the series, feel free to push this one ahead of the rest, as I am using it in my "Use more PCIe less PCI" series, which is mostly ACKed.