Removing the NT_STATUS_HAVE_NO_MEMORY_AND_FREE macro

[Andrew Bartlett]

On Sat, 2014-02-15 at 09:47 -0800, Jeremy Allison wrote:
> On Sat, Feb 15, 2014 at 03:21:11PM +0100, Kamen Mazdrashki wrote:
> > Hi list,
> > 
> > Perhaps I am missing something, but why we need this change to remove
> > NT_STATUS_HAVE_NO_MEMORY_AND_FREE? I understand there is
> > coding rule not to return from a macro, but in this case, this macro makes
> > the code easier to read.
> > 
> > Take for instance: libgpo/gpo_util.c:gpo_copy() function.
> > It is supposed to be just a simple copy-object implementation and using
> > NT_STATUS_HAVE_NO_MEMORY_AND_FREE it look straightforward
> > imho. But, now it is going to be bloated with 'if's all the way and this
> > will
> > literary double the length of it (which will make it hard to notice if you
> > have
> > missed to copy a struct field).
> > 
> > Just my 2c.
> 
> Well doing returns inside macros is generally bad
> programming practice, especially when you may need
> to do cleanups that get added later. Changes in
> control flow really need to be explicit, not implicit
> inside macros.
> 
> So it might make the code a little uglier, but it's
> then easier to maintain in the long run.
> 
> My 2c (and why I +1'ed it :-).

My starting position is that we should either have a coding style rule
and follow it, or not have the rule.  You might recall I opposed this
addition to the coding style - but not because I disagreed (I am
persuaded, particularly in the cases where the macro does not say
'RETURN' or 'assert'), but because I feel that we should make an effort
such as the one Garming has done to actually follow the new style at the
point we set the rule. 

Naturally in a project of our size the changes required to perfectly
follow our coding style, particularly around whitespace, would be huge
and disruptive, but when we have reasonable, reviewed patches to improve
compliance, we should either accept them or (as Kamen has proposed)
re-consider if the rule really does/should apply.

For Kamen's particular objection, I think routines such as that should
be auto-generated by putting the structure into IDL, and
ndr_pull_struct_blob and ndr_push_struct_blob used to do the copy. 

Andrew Bartlett

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