New python PIDL checks cause 2221 new Coverity warnings

Andrew Bartlett abartlet at samba.org
Fri Aug 28 00:04:33 UTC 2015


On Fri, 2015-08-28 at 09:47 +1200, Andrew Bartlett wrote:
> 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.

See attached.  Please review.

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: 0001-pidl-python-Calculate-maximum-integer-values-using-a.patch
Type: text/x-patch
Size: 2695 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150828/2f174240/0001-pidl-python-Calculate-maximum-integer-values-using-a-0001.bin>
-------------- 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/2f174240/signature-0001.sig>


More information about the samba-technical mailing list