[libvirt] [PATCH] virStringListLength: Ensure const correctness

[Cole Robinson]

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 at 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;
>          }

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