Re: [libvirt] [PATCH] virStringListLength: Ensure const correctness

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(a)redhat.com&gt; > --- > > 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; }