util: storage: Make @backingFormat optional in virStorageFileGetMetadataInternal

[libvirt] [PATCH] util: storage: Make @backingFormat optional in virStorageFileGetMetadataInternal

Posted by Peter Krempa 6 years, 10 months ago

Some callers don't need to know the backing format. Make the argument
optional by using a dummy int if NULL is passed.
---
 src/util/virstoragefile.c | 4 ++++
 src/util/virstoragefile.h | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 8047d977f..042698872 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -966,9 +966,13 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
                                   size_t len,
                                   int *backingFormat)
 {
+    int format;
     int ret = -1;
     size_t i;

+    if (!backingFormat)
+        backingFormat = &format;
+
     VIR_DEBUG("path=%s, buf=%p, len=%zu, meta->format=%d",
               meta->path, buf, len, meta->format);

diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index ce54a19ce..0bff8671f 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -293,7 +293,7 @@ int virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
                                       char *buf,
                                       size_t len,
                                       int *backingFormat)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

 virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path,
                                                     int fd,
-- 
2.12.2

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

Re: [libvirt] [PATCH] util: storage: Make @backingFormat optional in virStorageFileGetMetadataInternal

Posted by Jiri Denemark 6 years, 10 months ago

On Tue, Jun 20, 2017 at 16:37:21 +0200, Peter Krempa wrote:
> Some callers don't need to know the backing format. Make the argument
> optional by using a dummy int if NULL is passed.
> ---
>  src/util/virstoragefile.c | 4 ++++
>  src/util/virstoragefile.h | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)

ACK

Jirka

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

Re: [libvirt] [PATCH] util: storage: Make @backingFormat optional in virStorageFileGetMetadataInternal

Posted by Martin Kletzander 6 years, 10 months ago

On Tue, Jun 20, 2017 at 04:37:21PM +0200, Peter Krempa wrote:
>Some callers don't need to know the backing format. Make the argument
>optional by using a dummy int if NULL is passed.
>---
> src/util/virstoragefile.c | 4 ++++
> src/util/virstoragefile.h | 2 +-
> 2 files changed, 5 insertions(+), 1 deletion(-)
>

I wanted to say this is weird, even though it works, but then I saw it
already used in virStorageFileGetMetadataFrom{FD,Buf}(), so I guess it's
fine :)

>diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
>index 8047d977f..042698872 100644
>--- a/src/util/virstoragefile.c
>+++ b/src/util/virstoragefile.c
>@@ -966,9 +966,13 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
>                                   size_t len,
>                                   int *backingFormat)
> {
>+    int format;

I would change this to 'int dummy' so that it's clearly visible it won't
be used (and it's the same as in the other functions).

ACK either way.  Oh, I'm sorry,

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list