[libvirt] [PATCH 0/2] Yet another round of build fixes

[Michal Privoznik]

On 30.05.2016 16:56, Peter Krempa wrote:
> On Mon, May 30, 2016 at 15:53:08 +0200, Michal Privoznik wrote:
>> After these regular errors, I still cannot build cleanly:
>>
>> ../../src/conf/domain_conf.c: In function 'virDomainChrPreAlloc':
>> ../../src/conf/domain_conf.c:14109:57: error: potential null pointer dereference [-Werror=null-dereference]
>>      return VIR_REALLOC_N(*arrPtr, *cntPtr + 1);
>>                                                          ^
>> ../../src/conf/domain_conf.c: In function 'virDomainChrRemove':
>> ../../src/conf/domain_conf.c:14133:21: error: potential null pointer dereference [-Werror=null-dereference]
>>      for (i = 0; i < *cntPtr; i++) {
>>                      ^~~~~~~
>> cc1: all warnings being treated as errors
>>
>> These are obviously false positives. I know some developers here
>> are not very fond of fixing those, so my question is - is it
>> worth the effort or will my patches be NACKed straight away?
> 
> Looking at the code I'm not surprised that the compiler thinks that it's
> a null-deref. The only reason why it isn't is that we don't ever pass
> the VIR_DOMAIN_CHR_DEVICE_TYPE_LAST as device type and that's very hard
> to infer.
> 
> An option would be to remove the monstrosity called
> virDomainChrGetDomainPtrsInternal and friends and mabye produce a saner
> code that will be easier to introspect both for humans and compilers.

I guess that's matter of taste. I find the current code compact and easy
to read. What would be easier is if we dropped def->consoles,
def->serials, def->parallels ... in favour of one array, say def->chr
where all of those are merged together. But that's a bigger portion of
work, isn't it.

> 
>>
>> Michal Privoznik (2):
>>   apibuild: Substitute only pure number tokens
>>   virSocketAddrIsPrivate: Work on 32bits platforms
> 
> ACK to those two. This time I verified that I've applied the patch prior
> to trying to break it.

Thanks, pushed.

Michal