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 <arm...@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>> Acked-by: John Snow <js...@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.
