[PATCH 0/4] virerror: Make virReportEnumRangeError() check for type mismatch

Michal Privoznik posted 4 patches 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/cover.1695105934.git.mprivozn@redhat.com
src/conf/domain_validate.c       |  2 +-
src/util/virerror.h              | 14 ++++++++------
src/util/virnetdevvportprofile.c | 10 +++++-----
3 files changed, 14 insertions(+), 12 deletions(-)
[PATCH 0/4] virerror: Make virReportEnumRangeError() check for type mismatch
Posted by Michal Privoznik 9 months ago
There are few cases where virReportEnumRangeError() is passed to a
mismatching enum. With patch 4/4 we can check for this at compile time
(assuming -Wenum-compare is available).

For instance with clang I get:

FAILED: src/conf/libvirt_conf.a.p/domain_validate.c.o 
clang ...
../src/conf/domain_validate.c:184:9: error: comparison of different enumeration types ('virDomainVideoBackendType' and 'virDomainInputType') [-Werror,-Wenum-compare]
        virReportEnumRangeError(virDomainInputType, video->backend);
        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/util/virerror.h:175:47: note: expanded from macro 'virReportEnumRangeError'
                         (__typeof__(value))1 == (typname)1 && sizeof((typname)1) != 0 ? #typname : #typname)
                         ~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~

Whereas with gcc I get more obscure error:

FAILED: src/conf/libvirt_conf.a.p/domain_validate.c.o 
cc ...
In file included from ../src/util/virthread.h:25,
                 from ../src/util/virobject.h:25,
                 from ../src/conf/capabilities.h:28,
                 from ../src/conf/domain_conf.h:31,
                 from ../src/conf/domain_validate.h:25,
                 from ../src/conf/domain_validate.c:23:
../src/conf/domain_validate.c: In function ‘virDomainVideoDefValidate’:
../src/util/virerror.h:175:47: error: comparison between ‘enum <anonymous>’ and ‘enum <anonymous>’ [-Werror=enum-compare]
  175 |                          (__typeof__(value))1 == (typname)1 && sizeof((typname)1) != 0 ? #typname : #typname)
      |                                               ^~
../src/conf/domain_validate.c:184:9: note: in expansion of macro ‘virReportEnumRangeError’
  184 |         virReportEnumRangeError(virDomainInputType, video->backend);
      |         ^~~~~~~~~~~~~~~~~~~~~~~

But I guess this is also okay-ish - it reports there is a problem with
code.

NB this only works with variables that are of a certain enum types. It
does not work with those generic 'int var /* someEnum */' declarations,
understandably. But as we use virXMLPropEnum() more, we can declare
variables of their true type (enum) and get more and more calls to
virReportEnumRangeError() checked.

Michal Prívozník (4):
  virnetdevvportprofile: Turn virNetDevVPortProfileLinkOp enum into a
    proper typedef
  virNetDevVPortProfileOp8021Qbh: Use proper type in
    virReportEnumRangeError()
  virDomainVideoDefValidate: Use proper type in
    virReportEnumRangeError()
  virerror: Make virReportEnumRangeError() check for type mismatch

 src/conf/domain_validate.c       |  2 +-
 src/util/virerror.h              | 14 ++++++++------
 src/util/virnetdevvportprofile.c | 10 +++++-----
 3 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.41.0

Re: [PATCH 0/4] virerror: Make virReportEnumRangeError() check for type mismatch
Posted by Ján Tomko 9 months ago
On a Tuesday in 2023, Michal Privoznik wrote:
>There are few cases where virReportEnumRangeError() is passed to a
>mismatching enum. With patch 4/4 we can check for this at compile time
>(assuming -Wenum-compare is available).
>
>For instance with clang I get:
>
>FAILED: src/conf/libvirt_conf.a.p/domain_validate.c.o
>clang ...
>../src/conf/domain_validate.c:184:9: error: comparison of different enumeration types ('virDomainVideoBackendType' and 'virDomainInputType') [-Werror,-Wenum-compare]
>        virReportEnumRangeError(virDomainInputType, video->backend);
>        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>../src/util/virerror.h:175:47: note: expanded from macro 'virReportEnumRangeError'
>                         (__typeof__(value))1 == (typname)1 && sizeof((typname)1) != 0 ? #typname : #typname)
>                         ~~~~~~~~~~~~~~~~~~~~ ^  ~~~~~~~~~~
>
>Whereas with gcc I get more obscure error:
>
>FAILED: src/conf/libvirt_conf.a.p/domain_validate.c.o
>cc ...
>In file included from ../src/util/virthread.h:25,
>                 from ../src/util/virobject.h:25,
>                 from ../src/conf/capabilities.h:28,
>                 from ../src/conf/domain_conf.h:31,
>                 from ../src/conf/domain_validate.h:25,
>                 from ../src/conf/domain_validate.c:23:
>../src/conf/domain_validate.c: In function ‘virDomainVideoDefValidate’:
>../src/util/virerror.h:175:47: error: comparison between ‘enum <anonymous>’ and ‘enum <anonymous>’ [-Werror=enum-compare]
>  175 |                          (__typeof__(value))1 == (typname)1 && sizeof((typname)1) != 0 ? #typname : #typname)
>      |                                               ^~
>../src/conf/domain_validate.c:184:9: note: in expansion of macro ‘virReportEnumRangeError’
>  184 |         virReportEnumRangeError(virDomainInputType, video->backend);
>      |         ^~~~~~~~~~~~~~~~~~~~~~~
>
>But I guess this is also okay-ish - it reports there is a problem with
>code.
>
>NB this only works with variables that are of a certain enum types. It
>does not work with those generic 'int var /* someEnum */' declarations,
>understandably. But as we use virXMLPropEnum() more, we can declare
>variables of their true type (enum) and get more and more calls to
>virReportEnumRangeError() checked.
>
>Michal Prívozník (4):
>  virnetdevvportprofile: Turn virNetDevVPortProfileLinkOp enum into a
>    proper typedef
>  virNetDevVPortProfileOp8021Qbh: Use proper type in
>    virReportEnumRangeError()
>  virDomainVideoDefValidate: Use proper type in
>    virReportEnumRangeError()
>  virerror: Make virReportEnumRangeError() check for type mismatch
>
> src/conf/domain_validate.c       |  2 +-
> src/util/virerror.h              | 14 ++++++++------
> src/util/virnetdevvportprofile.c | 10 +++++-----
> 3 files changed, 14 insertions(+), 12 deletions(-)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Jano