[PATCH] A few cleanups

[Jeremy Allison]

On Mon, Feb 15, 2016 at 04:09:23PM +0100, Volker Lendecke wrote:
> On Mon, Feb 15, 2016 at 03:56:21PM +0100, Volker Lendecke wrote:
> > On Mon, Feb 15, 2016 at 03:34:41PM +0100, Andreas Schneider wrote:
> > >     bool ok;
> > > 
> > >     ok = ads_pull_uint32(ctx->ads, msg_internal, ...
> > >     if (ok) {
> > 
> > Last time I did this Jeremy took this pattern out of a patch of mine
> > before pushing, so I think this is not appropriate upstream anymore.
> > 
> > Can we clarify this topic once and for all?
> 
> For reference: It is 7b7aa016df35ed7f8388a9df08d66a816adc1bf7. The
> patch I had sent to Jeremy contained a helper variable for
> 
> if (!asn1_blob(asn1, &blob)) {
> 
> that did not make it into master. So this is at least controversial.

I removed it as every other check within the asn1.c file
was of the form:

  if (!fn_call()) {
    ... error handling...
  }

So Volker's change *in that file* stood out like a sore
thumb and looked strange (anyone reading through it
linearly would have seen a jarring discontinuity in
error handling).

Context *matters*.

Now, if you want to propose a patch to asn1.c
that replaces all these calls with the preferred
style in README.coding then feel free to do so,
and then the style will by consistent.

But I stand by my change in that specific case.