Re: [Qemu-devel] [PATCH v6 3/5] acpi: add build_append_namestring() helper

[Igor Mammedov]

On Wed, 28 Jan 2015 17:16:17 +0200
"Michael S. Tsirkin" <address@hidden> 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 <address@hidden>
> > Reviewed-by: Claudio Fontana <address@hidden>
> > Acked-by: Marcel Apfelbaum <address@hidden>
> > ---
> > 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 <address@hidden>
> > ---
> >  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()