On Tue, 22 Dec 2015 11:37:40 +0200
"Michael S. Tsirkin" <m...@redhat.com> wrote:
[...]
> > > > + for (i = 4; i <= 0xF; i++) {
> > > > + char *name = g_strdup_printf("_L0%X", i);
> > > > + method = aml_method(name, 0, AML_NOTSERIALIZED);
> > > > + aml_append(scope, method);
> > > > + g_free(name);
> > > > + }
> > >
> > > How about we make aml_method accept ... format instead?
> > actually instead of making aml_method(format,...) it would be
> > easier to make it accept Aml* so we could reuse aml_name(format,...)
> > in the end it would look like:
> >
> > Aml gpe_name = aml_name("_L0%X", i);
> > aml_method(gpe_name, AML_NOTSERIALIZED);
> >
> > in addition name object could be reused in other places
> > that reference that method.
>
> Except most methods are simple, so maybe do both APIs.
> Avoiding string duplication is a good idea,
> but using string constants works just as well.
I don't like having 2 APIs, one with 'Aml* name' and other
with 'const char *name' in C. I'd prefer to have just one
that suits the majority use cases and
I'm not so comfortable with using format string here directly
as it would look weird (at least to me):
aml_method("_L0%X", i, argcount, AML_NOTSERIALIZED);
- static format string check at compile time won't work here
or
aml_method(argcount, AML_NOTSERIALIZED, "_L0%X", i);
- that should be fine except of that argument order doesn't
match the way it's in spec, which I'd prefer to keep
so it leaves for me 2 options:
1: use aml_method(aml_name("_L0%X", i), argcount, AML_NOTSERIALIZED)
It's a bit of code churn and not sure if we really need it
but I can do if asked for it.
2: just leave it as is for now, because the most of method names
are just string constants. These "_L0%X" will be gone after
cleanup, leaving us with only handlers that use string const and
do some work.
I can replace g_strdup_printf() with static buffer here if you
dislike alloc/free in this patch.
