From: Johannes Sixt <j6t@kdbg.org>
Date: Tue, 17 May 2011 20:14:43 GMT
Subject: Re: [PATCH 1/3] connect: treat generic proxy processes like ssh processes
Message-ID: <4DD2D733.3070004@kdbg.org>
In-Reply-To: <20110517055422.GD10048@sigill.intra.peff.net>

Am 17.05.2011 07:54, schrieb Jeff King:
> On Mon, May 16, 2011 at 09:57:58PM +0200, Johannes Sixt wrote:
>> In my implementation, I xmalloced the pointer array and leaked it.

I noticed that it actually isn't leaked because finish_connect() frees
it. For this reason, I actually have to wonder why your version that
stored a pointer to automatic storage in ->argv worked.

> We also need to worry about the contents of each argv[] element, no? So
> we should be xstrdup()ing the host and port, which point into some
> string which gets passed to us. I didn't trace its provenance but I
> think it is better to be defensive.

I would not worry too much today. Of course, functions other than
start_command() might begin to access ->argv[i] with i > 0 later, but
then we have to audit all users of struct child_process anyway.
Currently, only start_command() uses these values, which is always
called at a time when they are still valid.

-- Hannes