[PATCH] Fix two CIDs

Simo simo at samba.org
Wed Nov 22 18:12:34 UTC 2017


On Wed, 2017-11-22 at 16:09 +0100, Volker Lendecke via samba-technical
wrote:
> On Wed, Nov 22, 2017 at 07:54:05AM -0700, David Mulder via samba-technical wrote:
> > 
> > On 11/21/2017 01:04 PM, Volker Lendecke via samba-technical wrote:
> > > Hi!
> > > 
> > > This NEWLY pushed file so severely needs overhaul to match
> > > README.Coding :-(
> > > 
> > > For example there are several if-statements without {} around the
> > > code. There's a reason why we don't want this: CVE 2014-1266, which
> > > was an early one with a famous name. Is this file so completely immune
> > > to any security-relevant exposure that this does not matter here? How
> > > have we verified that this is irrelevant to security?
> > 
> > I think security does matter here, since we're authenticating and
> > pulling info from the sysvol.
> > Not putting {} around if statements is a bad habit of mine.
> > > I'm not talking about the cosmetic 80-column thingy, something which
> > > this file does not follow either. I am talking about our way to
> > > protect from one aspect of security-aware coding, and a very easily
> > > implemented one.
> > 
> > Actually, if you set your tabwidth to 4 chars, the file abides by the
> > 80-column width (my bad).
> > Obviously that was a mistake.
> 
> This is not about you, none of this is. It's about our insufficent
> review process that does not catch them.
> 
> Sorry you got involved,


In MIT krb5 there is code that check every patch for adherence to their
coding style. (note that it is clever and checks only "changesets", it
does not complain for previous code of which they also had some
annoying out-of-style + change-of-style annoyances.

Maybe we should also just automate this so that people concentrated on
checking the actual code behavior do not have to waste time checking
the style "manually" ?

Simo.




More information about the samba-technical mailing list