New python PIDL checks cause 2221 new Coverity warnings

Andrew Bartlett abartlet at samba.org
Thu Aug 27 21:47:59 UTC 2015


On Thu, 2015-08-27 at 11:54 +0200, Stefan Metzmacher wrote:
> Hi Volker,
> 
> > > Finally, we should try to keep the solution as generic as 
> > > possible, as
> > > when we add new base types to PIDL, all these lists need to be 
> > > updated,
> > > and the person who is doing so probably won't remember the full
> > > background.
> > 
> > If you don't want to put ifdefs into generated code, can't we put 
> > that
> > into a handcrafted helper function and call that in the 
> > autogenerated
> > code?
> 
> That's a really good idea!
> 
> In such a helper function we could just avoid doing bit shifting
> at all.

I agree.  The bitshifting is ugly and technically undefined.  

> I really didn't like the horrible magic with bit shifting,
> but I verified it works and gave my review in order to fix the
> important replication bug.
> 
> But given these side effects, we really need to do something about 
> it!
> 
> const unsigned long long uint_max = (sizeof($target) == 8) ? 
> UINT64_MAX
> : (unsigned long long)((1ULL << (sizeof($target) * 8)) - 1);");
> 
> should be come:
> 
> const uintmax_t uint_max = ndr_sizeof2uintmax(sizeof($target));
> 
> uintmax_t ndr_sizeof2uintmax(size_t sizeof)
> {
> 
> 	switch (sizeof) {
> 	case 8:
> 		return INT64_MAX;
> 	case 4:
> 		return INT32_MAX:
> ...
> 	}
> 
> 	return 0;
> }

That is OK on the basis that Coverity doesn't mind that an initialiser
actually resolves to a constant value.  Indeed we would probably wish
to declare that as static inline to allow the compiler to do just that.

> A similar thing for ndr_sizeof2intmax().
> 
> Andrew, please provide patches to implement the above changes.

This is a reasonable, practical proposal, and I'm happy to do that
much.  It won't fix all the issues, but fixing the maximum value issues
will be a nice place to start.

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett
https://samba.org/~abartlet/
Authentication Developer, Samba Team         https://samba.org
Samba Development and Support, Catalyst IT   
https://catalyst.net.nz/services/samba




-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150828/8cf18a87/signature.sig>


More information about the samba-technical mailing list