From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54944) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YHbUZ-0000Zj-Kv for qemu-devel@nongnu.org; Sat, 31 Jan 2015 12:05:56 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YHbUW-0000wv-FQ for qemu-devel@nongnu.org; Sat, 31 Jan 2015 12:05:55 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43290) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YHbUW-0000wY-7Z for qemu-devel@nongnu.org; Sat, 31 Jan 2015 12:05:52 -0500 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0VH5n15023798 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Sat, 31 Jan 2015 12:05:50 -0500 Date: Sat, 31 Jan 2015 19:05:47 +0200 From: "Michael S. Tsirkin" Message-ID: <20150131170547.GA31871@redhat.com> References: <1422455690-13950-1-git-send-email-imammedo@redhat.com> <1422455690-13950-4-git-send-email-imammedo@redhat.com> <20150128151617.GD32474@redhat.com> <20150130124641.6fe7da6b@igors-macbook-pro.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150130124641.6fe7da6b@igors-macbook-pro.local> Subject: Re: [Qemu-devel] [PATCH v6 3/5] acpi: add build_append_namestring() helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org On Fri, Jan 30, 2015 at 12:46:41PM +0100, Igor Mammedov wrote: > On Wed, 28 Jan 2015 17:16:17 +0200 > "Michael S. Tsirkin" wrote: > > > On Wed, Jan 28, 2015 at 02:34:48PM +0000, Igor Mammedov wrote: > > > Use build_append_namestring() instead of build_append_nameseg() > > > So user won't have to care whether name is NameSeg, NamePath or > > > NameString. > > > > > > See for refernce ACPI 5.0: 20.2.2 Name Objects Encoding > > > > > > > typo > > > > > Signed-off-by: Igor Mammedov > > > Reviewed-by: Claudio Fontana > > > Acked-by: Marcel Apfelbaum > > > --- > > > v4: > > > * fix codding style of ACPI_DualNamePrefix & ACPI_MultiNamePrefix > > > it's imposible to use literals as suggested due to > > > g_array_append_val() requiring reference value > > > > > > v3: > > > assert on wrong Segcount earlier and extend condition to > > > seg_count > 0 && seg_count <= 255 > > > as suggested by Claudio Fontana > > > --- > > > hw/acpi/aml-build.c | 92 > > > ++++++++++++++++++++++++++++++++++++++++++++- > > > hw/i386/acpi-build.c | 35 ++++++++--------- > > > include/hw/acpi/aml-build.h | 2 +- 3 files changed, 108 > > > insertions(+), 21 deletions(-) > > > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > > index b28c1f4..ed4ab56 100644 > > > --- a/hw/acpi/aml-build.c > > > +++ b/hw/acpi/aml-build.c > > > @@ -52,7 +52,7 @@ void build_append_array(GArray *array, GArray > > > *val) > > > #define ACPI_NAMESEG_LEN 4 > > > > > > -void GCC_FMT_ATTR(2, 3) > > > +static void GCC_FMT_ATTR(2, 3) > > > build_append_nameseg(GArray *array, const char *format, ...) > > > { > > > /* It would be nicer to use g_string_vprintf but it's only > > > there in 2.22 */ @@ -71,6 +71,96 @@ build_append_nameseg(GArray > > > *array, const char *format, ...) g_array_append_vals(array, "____", > > > ACPI_NAMESEG_LEN - len); } > > > > > > +static void > > > +build_append_namestringv(GArray *array, const char *format, > > > va_list ap) +{ > > > + /* It would be nicer to use g_string_vprintf but it's only > > > there in 2.22 */ > > > + char *s; > > > + int len; > > > + va_list va_len; > > > + char **segs; > > > + char **segs_iter; > > > + int seg_count = 0; > > > + > > > + va_copy(va_len, ap); > > > + len = vsnprintf(NULL, 0, format, va_len); > > > + va_end(va_len); > > > + len += 1; > > > + s = g_new(typeof(*s), len); > > > + > > > + len = vsnprintf(s, len, format, ap); > > > + > > > + segs = g_strsplit(s, ".", 0); > > > + g_free(s); > > > + > > > + /* count segments */ > > > + segs_iter = segs; > > > + while (*segs_iter) { > > > + ++segs_iter; > > > + ++seg_count; > > > > > How about we split out formatting? > > Make +build_append_namestringv acceptconst char **segs, int nsegs, > > put all code above to build_append_namestring. > Can't do this, AML API calls which accept format string will > use build_append_namestringv() OK but I still think it's a good idea to split this out, and have a static function that accepts an array of segments.