[libvirt] [PATCH 03/10] conf: domaincaps: Fix broken attempt at being const-correct

Peter Krempa posted 10 patches 6 years, 2 months ago
[libvirt] [PATCH 03/10] conf: domaincaps: Fix broken attempt at being const-correct
Posted by Peter Krempa 6 years, 2 months ago
'virBlahPtr const blah' results into modification to the value of 'blah'
triggering compilation error rather than the modification of the virBlah
struct the pointer points to.

All of the domain capability formatting code was broken in this regard.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/conf/domain_capabilities.c | 32 ++++++++++++++++----------------
 src/conf/domain_capabilities.h |  4 ++--
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
index 09bf047647..b9de4bfe5d 100644
--- a/src/conf/domain_capabilities.c
+++ b/src/conf/domain_capabilities.c
@@ -321,7 +321,7 @@ virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum)

 static int
 virDomainCapsEnumFormat(virBufferPtr buf,
-                        virDomainCapsEnumPtr capsEnum,
+                        const virDomainCapsEnum *capsEnum,
                         const char *capsEnumName,
                         virDomainCapsValToStr valToStr)
 {
@@ -362,7 +362,7 @@ virDomainCapsEnumFormat(virBufferPtr buf,

 static void
 virDomainCapsStringValuesFormat(virBufferPtr buf,
-                                virDomainCapsStringValuesPtr values)
+                                const virDomainCapsStringValues *values)
 {
     size_t i;

@@ -406,7 +406,7 @@ virDomainCapsStringValuesFormat(virBufferPtr buf,

 static void
 virDomainCapsLoaderFormat(virBufferPtr buf,
-                          virDomainCapsLoaderPtr loader)
+                          const virDomainCapsLoader *loader)
 {
     FORMAT_PROLOGUE(loader);

@@ -420,9 +420,9 @@ virDomainCapsLoaderFormat(virBufferPtr buf,

 static void
 virDomainCapsOSFormat(virBufferPtr buf,
-                      virDomainCapsOSPtr os)
+                      const virDomainCapsOS *os)
 {
-    virDomainCapsLoaderPtr loader = &os->loader;
+    const virDomainCapsLoader *loader = &os->loader;

     FORMAT_PROLOGUE(os);

@@ -453,7 +453,7 @@ virDomainCapsCPUCustomFormat(virBufferPtr buf,

 static void
 virDomainCapsCPUFormat(virBufferPtr buf,
-                       virDomainCapsCPUPtr cpu)
+                       const virDomainCapsCPU *cpu)
 {
     virBufferAddLit(buf, "<cpu>\n");
     virBufferAdjustIndent(buf, 2);
@@ -492,7 +492,7 @@ virDomainCapsCPUFormat(virBufferPtr buf,

 static void
 virDomainCapsDeviceDiskFormat(virBufferPtr buf,
-                              virDomainCapsDeviceDiskPtr const disk)
+                              const virDomainCapsDeviceDisk *disk)
 {
     FORMAT_PROLOGUE(disk);

@@ -506,7 +506,7 @@ virDomainCapsDeviceDiskFormat(virBufferPtr buf,

 static void
 virDomainCapsDeviceGraphicsFormat(virBufferPtr buf,
-                                  virDomainCapsDeviceGraphicsPtr const graphics)
+                                  const virDomainCapsDeviceGraphics *graphics)
 {
     FORMAT_PROLOGUE(graphics);

@@ -518,7 +518,7 @@ virDomainCapsDeviceGraphicsFormat(virBufferPtr buf,

 static void
 virDomainCapsDeviceVideoFormat(virBufferPtr buf,
-                               virDomainCapsDeviceVideoPtr const video)
+                               const virDomainCapsDeviceVideo *video)
 {
     FORMAT_PROLOGUE(video);

@@ -530,7 +530,7 @@ virDomainCapsDeviceVideoFormat(virBufferPtr buf,

 static void
 virDomainCapsDeviceHostdevFormat(virBufferPtr buf,
-                                 virDomainCapsDeviceHostdevPtr const hostdev)
+                                 const virDomainCapsDeviceHostdev *hostdev)
 {
     FORMAT_PROLOGUE(hostdev);

@@ -546,7 +546,7 @@ virDomainCapsDeviceHostdevFormat(virBufferPtr buf,

 static void
 virDomainCapsDeviceRNGFormat(virBufferPtr buf,
-                             virDomainCapsDeviceRNGPtr const rng)
+                             const virDomainCapsDeviceRNG *rng)
 {
     FORMAT_PROLOGUE(rng);

@@ -575,7 +575,7 @@ virDomainCapsDeviceRNGFormat(virBufferPtr buf,
  */
 static void
 virDomainCapsFeatureGICFormat(virBufferPtr buf,
-                              virDomainCapsFeatureGICPtr const gic)
+                              const virDomainCapsFeatureGIC *gic)
 {
     FORMAT_PROLOGUE(gic);

@@ -586,7 +586,7 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf,

 static void
 virDomainCapsFeatureSEVFormat(virBufferPtr buf,
-                              virSEVCapabilityPtr const sev)
+                              const virSEVCapability *sev)
 {
     if (!sev) {
         virBufferAddLit(buf, "<sev supported='no'/>\n");
@@ -605,7 +605,7 @@ virDomainCapsFeatureSEVFormat(virBufferPtr buf,


 char *
-virDomainCapsFormat(virDomainCapsPtr const caps)
+virDomainCapsFormat(const virDomainCaps *caps)
 {
     virBuffer buf = VIR_BUFFER_INITIALIZER;
     const char *virttype_str = virDomainVirtTypeToString(caps->virttype);
@@ -669,7 +669,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)


 static int
-virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps,
+virDomainCapsDeviceRNGDefValidate(const virDomainCaps *caps,
                                   const virDomainRNGDef *dev)
 {
     if (ENUM_VALUE_MISSING(caps->rng.model, dev->model)) {
@@ -683,7 +683,7 @@ virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps,


 int
-virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps,
+virDomainCapsDeviceDefValidate(const virDomainCaps *caps,
                                const virDomainDeviceDef *dev,
                                const virDomainDef *def G_GNUC_UNUSED)
 {
diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
index a31458c653..6b27eac11f 100644
--- a/src/conf/domain_capabilities.h
+++ b/src/conf/domain_capabilities.h
@@ -226,9 +226,9 @@ int virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum,
                          unsigned int *values);
 void virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum);

-char * virDomainCapsFormat(virDomainCapsPtr const caps);
+char * virDomainCapsFormat(const virDomainCaps *caps);

-int virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps,
+int virDomainCapsDeviceDefValidate(const virDomainCaps *caps,
                                    const virDomainDeviceDef *dev,
                                    const virDomainDef *def);

-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 03/10] conf: domaincaps: Fix broken attempt at being const-correct
Posted by Daniel P. Berrangé 6 years, 2 months ago
On Tue, Nov 12, 2019 at 08:27:40AM +0100, Peter Krempa wrote:
> 'virBlahPtr const blah' results into modification to the value of 'blah'
> triggering compilation error rather than the modification of the virBlah
> struct the pointer points to.

Shall we add a syntax check rule to forbid "virFOOPtr const", and
"const virFOOPtr".

Indeed, I rather wish we never had the "Ptr" suffix at all, and
just used a "*" normally, but that would be a too huge change.

> 
> All of the domain capability formatting code was broken in this regard.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/conf/domain_capabilities.c | 32 ++++++++++++++++----------------
>  src/conf/domain_capabilities.h |  4 ++--
>  2 files changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 09bf047647..b9de4bfe5d 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
> @@ -321,7 +321,7 @@ virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum)
> 
>  static int
>  virDomainCapsEnumFormat(virBufferPtr buf,
> -                        virDomainCapsEnumPtr capsEnum,
> +                        const virDomainCapsEnum *capsEnum,
>                          const char *capsEnumName,
>                          virDomainCapsValToStr valToStr)
>  {
> @@ -362,7 +362,7 @@ virDomainCapsEnumFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsStringValuesFormat(virBufferPtr buf,
> -                                virDomainCapsStringValuesPtr values)
> +                                const virDomainCapsStringValues *values)
>  {
>      size_t i;
> 
> @@ -406,7 +406,7 @@ virDomainCapsStringValuesFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsLoaderFormat(virBufferPtr buf,
> -                          virDomainCapsLoaderPtr loader)
> +                          const virDomainCapsLoader *loader)
>  {
>      FORMAT_PROLOGUE(loader);
> 
> @@ -420,9 +420,9 @@ virDomainCapsLoaderFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsOSFormat(virBufferPtr buf,
> -                      virDomainCapsOSPtr os)
> +                      const virDomainCapsOS *os)
>  {
> -    virDomainCapsLoaderPtr loader = &os->loader;
> +    const virDomainCapsLoader *loader = &os->loader;
> 
>      FORMAT_PROLOGUE(os);
> 
> @@ -453,7 +453,7 @@ virDomainCapsCPUCustomFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsCPUFormat(virBufferPtr buf,
> -                       virDomainCapsCPUPtr cpu)
> +                       const virDomainCapsCPU *cpu)
>  {
>      virBufferAddLit(buf, "<cpu>\n");
>      virBufferAdjustIndent(buf, 2);
> @@ -492,7 +492,7 @@ virDomainCapsCPUFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsDeviceDiskFormat(virBufferPtr buf,
> -                              virDomainCapsDeviceDiskPtr const disk)
> +                              const virDomainCapsDeviceDisk *disk)
>  {
>      FORMAT_PROLOGUE(disk);
> 
> @@ -506,7 +506,7 @@ virDomainCapsDeviceDiskFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsDeviceGraphicsFormat(virBufferPtr buf,
> -                                  virDomainCapsDeviceGraphicsPtr const graphics)
> +                                  const virDomainCapsDeviceGraphics *graphics)
>  {
>      FORMAT_PROLOGUE(graphics);
> 
> @@ -518,7 +518,7 @@ virDomainCapsDeviceGraphicsFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsDeviceVideoFormat(virBufferPtr buf,
> -                               virDomainCapsDeviceVideoPtr const video)
> +                               const virDomainCapsDeviceVideo *video)
>  {
>      FORMAT_PROLOGUE(video);
> 
> @@ -530,7 +530,7 @@ virDomainCapsDeviceVideoFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsDeviceHostdevFormat(virBufferPtr buf,
> -                                 virDomainCapsDeviceHostdevPtr const hostdev)
> +                                 const virDomainCapsDeviceHostdev *hostdev)
>  {
>      FORMAT_PROLOGUE(hostdev);
> 
> @@ -546,7 +546,7 @@ virDomainCapsDeviceHostdevFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsDeviceRNGFormat(virBufferPtr buf,
> -                             virDomainCapsDeviceRNGPtr const rng)
> +                             const virDomainCapsDeviceRNG *rng)
>  {
>      FORMAT_PROLOGUE(rng);
> 
> @@ -575,7 +575,7 @@ virDomainCapsDeviceRNGFormat(virBufferPtr buf,
>   */
>  static void
>  virDomainCapsFeatureGICFormat(virBufferPtr buf,
> -                              virDomainCapsFeatureGICPtr const gic)
> +                              const virDomainCapsFeatureGIC *gic)
>  {
>      FORMAT_PROLOGUE(gic);
> 
> @@ -586,7 +586,7 @@ virDomainCapsFeatureGICFormat(virBufferPtr buf,
> 
>  static void
>  virDomainCapsFeatureSEVFormat(virBufferPtr buf,
> -                              virSEVCapabilityPtr const sev)
> +                              const virSEVCapability *sev)
>  {
>      if (!sev) {
>          virBufferAddLit(buf, "<sev supported='no'/>\n");
> @@ -605,7 +605,7 @@ virDomainCapsFeatureSEVFormat(virBufferPtr buf,
> 
> 
>  char *
> -virDomainCapsFormat(virDomainCapsPtr const caps)
> +virDomainCapsFormat(const virDomainCaps *caps)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
>      const char *virttype_str = virDomainVirtTypeToString(caps->virttype);
> @@ -669,7 +669,7 @@ virDomainCapsFormat(virDomainCapsPtr const caps)
> 
> 
>  static int
> -virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps,
> +virDomainCapsDeviceRNGDefValidate(const virDomainCaps *caps,
>                                    const virDomainRNGDef *dev)
>  {
>      if (ENUM_VALUE_MISSING(caps->rng.model, dev->model)) {
> @@ -683,7 +683,7 @@ virDomainCapsDeviceRNGDefValidate(virDomainCapsPtr const caps,
> 
> 
>  int
> -virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps,
> +virDomainCapsDeviceDefValidate(const virDomainCaps *caps,
>                                 const virDomainDeviceDef *dev,
>                                 const virDomainDef *def G_GNUC_UNUSED)
>  {
> diff --git a/src/conf/domain_capabilities.h b/src/conf/domain_capabilities.h
> index a31458c653..6b27eac11f 100644
> --- a/src/conf/domain_capabilities.h
> +++ b/src/conf/domain_capabilities.h
> @@ -226,9 +226,9 @@ int virDomainCapsEnumSet(virDomainCapsEnumPtr capsEnum,
>                           unsigned int *values);
>  void virDomainCapsEnumClear(virDomainCapsEnumPtr capsEnum);
> 
> -char * virDomainCapsFormat(virDomainCapsPtr const caps);
> +char * virDomainCapsFormat(const virDomainCaps *caps);
> 
> -int virDomainCapsDeviceDefValidate(virDomainCapsPtr const caps,
> +int virDomainCapsDeviceDefValidate(const virDomainCaps *caps,
>                                     const virDomainDeviceDef *dev,
>                                     const virDomainDef *def);
> 
> -- 
> 2.23.0
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 03/10] conf: domaincaps: Fix broken attempt at being const-correct
Posted by Ján Tomko 6 years, 2 months ago
On Tue, Nov 12, 2019 at 09:30:14AM +0000, Daniel P. Berrangé wrote:
>On Tue, Nov 12, 2019 at 08:27:40AM +0100, Peter Krempa wrote:
>> 'virBlahPtr const blah' results into modification to the value of 'blah'
>> triggering compilation error rather than the modification of the virBlah
>> struct the pointer points to.
>
>Shall we add a syntax check rule to forbid "virFOOPtr const", and
>"const virFOOPtr".
>

We already do have a rule against const virFOOPtr.

Jano

>Indeed, I rather wish we never had the "Ptr" suffix at all, and
>just used a "*" normally, but that would be a too huge change.
>
>>
>> All of the domain capability formatting code was broken in this regard.
>>
>> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
>> ---
>>  src/conf/domain_capabilities.c | 32 ++++++++++++++++----------------
>>  src/conf/domain_capabilities.h |  4 ++--
>>  2 files changed, 18 insertions(+), 18 deletions(-)
>>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list