On 02/09/2016 01:19 PM, Andrea Bolognani wrote: > 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; > } Since this is breaking the default build for me, I squashed in that hunk and settled on '(cast)variable' spacing and pushed this patch. Thanks, Cole -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list