Re: [PATCH] virStringListLength: Ensure const correctness

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2016-02-09 at 18:18 +0100, Michal Privoznik wrote:
> The virStringListLength function does not ever modify the passed
> string list. It merely counts the items in it. Make sure that we
> reflect this bit in the function header.
> 
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
> 
> This is hugely driven by a compilation error observed after the latest Martin
> revert (ea913d185df9). I'm seeing this compilation error:
> 
> util/virpolkit.c: In function 'virPolkitCheckAuth':
> util/virpolkit.c:93:47: error: passing argument 1 of 'virStringListLength' from incompatible pointer type [-Werror]
>                            virStringListLength(details) / 2,
>                                                ^
> In file included from util/virpolkit.c:33:0:
> util/virstring.h:204:8: note: expected 'char **' but argument is of type 'const char **'
>  size_t virStringListLength(char **strings);
> 
> But for some reason, implicit typecast from char ** to const char ** nor const
> char * const * is allowed by gcc. I don't really understand why, so if anybody
> has some explanation, please do explain.

Nitpick: you're using both

  (const char * const *) var

and

  (const char * const *)var

in your patch: both are used across libvirt's codebase, so please
pick one and stick to that.

I've looked for answers on the Internet, as you do, and I've stumbled
upon this entry from the C FAQ:

  http://c-faq.com/ansi/constmismatch.html

Now, having to explicitly cast when calling the function definitely
sucks, but I don't really see a way around it at the moment; plus
doing the opposite and casting a const pointer to a non-const pointer
looks like a step in the wrong direction.

That said, I'd like to hear more opinions so weak ACK with the
whitespace inconsistency fixed and the following squashed in.

Cheers.


diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c
index 1200813..264ccc8 100644
--- a/src/storage/storage_backend_sheepdog.c
+++ b/src/storage/storage_backend_sheepdog.c
@@ -167,7 +167,8 @@ virStorageBackendSheepdogRefreshAllVol(virConnectPtr conn ATTRIBUTE_UNUSED,
 
         cells = virStringSplit(line, " ", 0);
 
-        if (cells != NULL && virStringListLength(cells) > 2) {
+        if (cells != NULL &&
+            virStringListLength((const char * const *) cells) > 2) {
             if (virStorageBackendSheepdogAddVolume(conn, pool, cells[1]) < 0)
                 goto cleanup;
         }
-- 
Andrea Bolognani
Software Engineer - Virtualization Team

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]