Re: [PATCH 5/9] merge-recursive: avoid returning a wholesale struct

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 1, 2016, 13:48 UTC
Message-ID: <alpine.DEB.2.20.1607011542210.12947@virtualbox>
In-Reply-To: <xmqqr3bf3fvr.fsf@gitster.mtv.corp.google.com>

Hi Junio,

On Wed, 29 Jun 2016, Junio C Hamano wrote:

Show 7 quoted lines

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > It is technically allowed, as per C89, for functions' return type to
> > be complete structs (i.e. *not* just pointers to structs), but it is
> > bad practice.
> 
> Not necessarily.

Okay, I fixed it.

Show 10 quoted lines

> > This is a very late attempt to contain the damage done by this developer
> > in 6d297f8 (Status update on merge-recursive in C, 2006-07-08) which
> > introduced such a return type.
> >
> > It will also help the current effort to libify merge-recursive.c, as
> > it will allow us to return proper error codes later.
> 
> But this part of the motivation does make sense.  Having to pass an
> extra int &error_code field, only because the return value is
> already used for something else, is a lot more weird.

While I still think it is inelegant to return a whole struct (understanding the machine code aspects of it), I toned it down to say that we just do not use that construct in Git's source code. And I kept the paragraph that you found more convincing, of course.

Ciao, Dscho

P.S.: If it is not too much of a problem, may I ask you to simply delete remainders of my patches when replying and not commenting on them? I just deleted 226 lines after verifying that you really did not respond to any part of it in the unhelpful Alpine client (which I still use because it *still* fits much better with my workflow than anything else, by a lot). Again, not a big deal if it would make your life more painful.