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" <m...@redhat.com> 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 <imamm...@redhat.com>
> > > Reviewed-by: Claudio Fontana <claudio.font...@huawei.com>
> > > Acked-by: Marcel Apfelbaum <mar...@redhat.com>
> > > ---
> > > 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 <claudio.font...@huawei.com>
> > > ---
> > > 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.
