On 10/29/21 17:28, Eric Blake wrote:
On Thu, Oct 28, 2021 at 12:25:17PM +0200, Markus Armbruster wrote:
The code to check command policy can see special feature flag 'deprecated' as command flag QCO_DEPRECATED. I want to make feature flag 'unstable' visible there as well, so I can add policy for it.
To let me make it visible, add member @special_features (a bitset of QapiSpecialFeature) to QmpCommand, and adjust the generator to pass it through qmp_register_command(). Then replace "QCO_DEPRECATED in @flags" by QAPI_DEPRECATED in @special_features", and drop QCO_DEPRECATED.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Acked-by: John Snow <jsnow@redhat.com> ---
+++ b/qapi/qmp-dispatch.c @@ -176,7 +176,7 @@ QDict *qmp_dispatch(const QmpCommandList *cmds, QObject *request, "The command %s has not been found", command); goto out; } - if (cmd->options & QCO_DEPRECATED) { + if (cmd->special_features & 1u << QAPI_DEPRECATED) {
I admit having to check the C operator precedence table when reading
This doesn't seem a good use of (y)our time. Using a pair of parenthesis is simpler. I expect in a not far future that compilers emit a warning for this.
this (<< is higher than &); if writing it myself, I would probably have used explicit () to avoid reviewer confusion, but what you have is correct. (After grepping for ' & 1.*<<' and ' & (1.*<<', it looks like authors using explicit precedence happens more often, but that there are other instances in the code base relying on implicit precedence.)
$ git grep -E ' & [0-9a-zA-Z_]+ <<' hw/dma/pl330.c:997: if (~ch->parent->inten & ch->parent->ev_status & 1 << ev_id) { hw/dma/xlnx-zynq-devcfg.c:198: if (s->regs[R_LOCK] & 1 << i) { hw/intc/grlib_irqmp.c:144: if (s->broadcast & 1 << irq) { hw/net/fsl_etsec/rings.c:491: if (etsec->regs[RSTAT].value & 1 << (23 - ring_nbr)) { hw/net/virtio-net.c:748: (n->host_features & 1ULL << VIRTIO_NET_F_MTU)) { hw/pci-host/mv64361.c:812: if ((ch & 0xff << i) && !(val & 0xff << i)) { hw/pci-host/mv64361.c:858: if (s->gpp_int_level && !(val & 0xff << b)) { hw/ssi/xilinx_spi.c:123: qemu_set_irq(s->cs_lines[i], !(~s->regs[R_SPISSR] & 1 << i)); hw/ssi/xilinx_spips.c:441: r[idx[!d]] |= x[idx[d]] & 1 << bit[d] ? 1 << bit[!d] : 0; target/s390x/cpu_features.c:56: if (init[i] & 1ULL << j) { tests/qtest/bios-tables-test.c:209: if (!(val & 1UL << 20 /* HW_REDUCED_ACPI */)) { Not that many.