New python PIDL checks cause 2221 new Coverity warnings

Kai Blin kai at samba.org
Wed Aug 26 12:02:02 UTC 2015


On 2015-08-26 00:24, Andrew Bartlett wrote:

Hi Andrew,

> Yes, we expected the compiler to optimise this out.  That was a
> deliberate design pattern to avoid trying to teach PIDL the size and
> maximum values of 'long' and all the IDL types (both classes of which
> change size depending on the architecture), and instead chose to rely
> on the compiler to handle that.

I'm a bit confused by this. On the first reading, I thought you meant 
that PIDL data structure sizes were architecture-dependent. Which then 
would imply that the network traffic produced was architecture-dependent 
as well. As that doesn't really make any sense, I take it you meant 
something else, and I guess it is "depending on the architecture, we 
need different types in order to fit the IDL types". That in turn sounds 
like something we want configure to sort out for us, instead of writing 
(potentially undefined) run-time checks and hoping for the compiler to 
optimize it out.


> The idea was that consistent code across all the types and
> architectures (duplicated only for signed/unsigned in one area), and so
> relying on the compiler to do it's work would be smarter than encoding
> the compiler's knowledge in PIDL, and risking getting that wrong.

I'm not sure "consistent code" is worth drowning out the Coverity 
results in a noise of false positives (or rather true positives that we 
want to ignore). We could autogenerate the size tables during configure, 
I guess.

> The biggest challenge is that a number of cases only really come up on
> 32 bit system, where a python long can't hold a uint32_t (on 64 bit it
> can), and so PyInt_FromLong can fail.

Who's still using 32bit systems these days? Only my ARM boxes still are 
32bit. :) Anyway, I'm still not convinced that this isn't exactly what 
should be figured out during configure.

> In short, when writing it we didn't consider Coverity giving warnings
> about this, we just expected the compiler to optimise it.

I see no point in crying over spilled milk. So now that the plan has 
collided with reality, how do we proceed to make our Coverity useful again?

Cheers,
Kai

-- 
Kai Blin
Samba Developer http://samba.team



More information about the samba-technical mailing list