Re: [PATCH v2 3/7] rebase: add support for multiple hooks

From: "brian m. carlson" <sandals@crustytoothpaste.net>
To: Duy Nguyen <pclouds@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
	"Jeff King" <peff@peff.net>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Junio C Hamano" <gitster@pobox.com>,
	"Johannes Sixt" <j6t@kdbg.org>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Jonathan Nieder" <jrnieder@gmail.com>
Subject: Re: [PATCH v2 3/7] rebase: add support for multiple hooks
Date: Wed, 15 May 2019 22:55:37 +0000	[thread overview]
Message-ID: <20190515225537.GL7458@genre.crustytoothpaste.net> (raw)
In-Reply-To: <CACsJy8CnCoMK7ahjRotKfiC2ic-U0hdhKuQAeF-TtVt8N+bU2Q@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 913 bytes --]

On Tue, May 14, 2019 at 07:56:49PM +0700, Duy Nguyen wrote:
> On Tue, May 14, 2019 at 7:24 AM brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > -       close(cp.in);
> 
> In the old code, we close cp.in...
> 
> > +int post_rewrite_rebase_hook(const char *name, const char *path, void *input)
> > +{
> > +       struct child_process child = CHILD_PROCESS_INIT;
> > +
> > +       child.in = open(input, O_RDONLY);
> > +       child.stdout_to_stderr = 1;
> > +       child.trace2_hook_name = "post-rewrite";
> 
> maybe use "name" and avoid hard coding "post-rewrite".
> 
> > +       argv_array_push(&child.args, path);
> > +       argv_array_push(&child.args, "rebase");
> > +       return run_command(&child);
> 
> ... but in the new one we don't. Smells fd leaking to me.

Ah, good point.  Will fix.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]