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