From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:40508)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1YqOVz-0004Gp-20
	for qemu-devel@nongnu.org; Thu, 07 May 2015 12:19:11 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1YqOVu-0003a3-H7
	for qemu-devel@nongnu.org; Thu, 07 May 2015 12:19:11 -0400
Received: from mailapp01.imgtec.com ([195.59.15.196]:58227)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Leon.Alrae@imgtec.com>) id 1YqOVu-0003Xh-BU
	for qemu-devel@nongnu.org; Thu, 07 May 2015 12:19:06 -0400
Message-ID: <554B9072.8060509@imgtec.com>
Date: Thu, 7 May 2015 17:18:58 +0100
From: Leon Alrae <leon.alrae@imgtec.com>
MIME-Version: 1.0
References: <1430999376-16601-1-git-send-email-leon.alrae@imgtec.com>
	<1430999376-16601-3-git-send-email-leon.alrae@imgtec.com>
	<CAFEAcA8bGU8dw9sabZb46C+paKJBPC0xK1bX8Wq6wGAHrGpeFA@mail.gmail.com>
In-Reply-To: <CAFEAcA8bGU8dw9sabZb46C+paKJBPC0xK1bX8Wq6wGAHrGpeFA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH v2 2/2] semihosting: add
	--semihosting-config arg sub-argument
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Liviu Ionescu <ilg@livius.net>, Christopher Covington <christopher.covington@linaro.org>, QEMU Developers <qemu-devel@nongnu.org>, Matthew Fortune <matthew.fortune@imgtec.com>

On 07/05/2015 14:02, Peter Maydell wrote:
>> +static int add_semihosting_arg(const char *name, const char *val, void *opaque)
>> +{
>> +    SemihostingConfig *s = opaque;
>> +    if (strcmp(name, "arg") == 0) {
>> +        s->argc++;
>> +        s->argv = g_realloc(s->argv, s->argc * sizeof(void *));
>> +        s->argv[s->argc - 1] = val;
>> +    }
> 
> Consider using a glib pointer array? Then this is just
> a call to g_pointer_array_add().

glib pointer array certainly simplifies managing an array of pointers,
but I think in this case we wouldn't benefit much from it as the array
is set here only and never modified later. Also people not familiar with
glib pointer arrays may find raw int argc and const char **argv fields
clearer than GPtrArray type.
I'll leave it as it is, but if anyone has strong preference on using
g_ptr_array I don't mind updating the patch as changes are trivial.

Thanks,
Leon