W_ERROR_HAVE_NO_MEMORY and friends

[Michael Adam]

Andrew Bartlett wrote:
> On Wed, 2012-05-30 at 12:35 +0200, Kai Blin wrote:
> > On 2012-05-30 07:59, Volker Lendecke wrote:
> > 
> > >>        via  6a1ad76 s4-dns: Use W_ERROR_HAVE_NO_MEMORY in create_response_rr
> > > 
> > > Where is the rule laid down that we must use those macros?
> > 
> > I'm not aware of any rule, but the rest of the DNS server code is using
> > that macro, so I thought consistency was a good thing.
> > 
> > > I find them completely obnoxious to use. Doing a return or
> > > goto from something that looks like a function call is just
> > > wrong to me. To save precious screen space, I would rather
> > > go and introduce a special rule to say
> > > 
> > > if (ptr == NULL) { return WERR_NOMEM };
> > 
> > This is not about screen space for me, it's about having to write less
> > boilerplate code. Code completion works better on the macro.
> 
> I've written code that uses it, and code that does not.  One of the
> advantages is that it promotes efficient and correct error handling
> (that is, checking every error, perhaps freeing a temp mem context). 
> 
> I tend to find that code that uses this has correct error handling,
> while code that doesn't (and so has the manually expanded version) tends
> to have poorer handling, often missing details. 

Some additional thoughts.

It is true that the code using the macros is more compact,
and hence possibly easier to read and grasp at first sight.
I also liked the macros quite a lot. But while it might be
easier to start off with a better error handling, this
might also be deceiving safety in some situations, since
the automatic use of these macros might make you forget
some special error handling (freeing of memory, whatnot)
that needs to be done. So currently I tend to agree with
Volker that the values in the compactness of code do not
make up for the additional sublte dangers and possible
misunderstandings in the code.

Cheers - Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 206 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20120530/f8679477/attachment.pgp>