From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52543) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBOQu-0006pa-3C for qemu-devel@nongnu.org; Tue, 22 Dec 2015 10:01:04 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBOQp-00030W-0I for qemu-devel@nongnu.org; Tue, 22 Dec 2015 10:01:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50731) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBOQo-00030K-PD for qemu-devel@nongnu.org; Tue, 22 Dec 2015 10:00:54 -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 (Postfix) with ESMTPS id 33B952FB8C6 for ; Tue, 22 Dec 2015 15:00:54 +0000 (UTC) Date: Tue, 22 Dec 2015 16:00:52 +0100 From: Igor Mammedov Message-ID: <20151222160052.179e9999@nial.brq.redhat.com> In-Reply-To: <20151222164436-mutt-send-email-mst@redhat.com> References: <1449704528-289297-1-git-send-email-imammedo@redhat.com> <1449704528-289297-55-git-send-email-imammedo@redhat.com> <20151219222828-mutt-send-email-mst@redhat.com> <20151221163523.0a35870b@nial.brq.redhat.com> <20151222113548-mutt-send-email-mst@redhat.com> <20151222153824.13dcff98@nial.brq.redhat.com> <20151222164436-mutt-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 54/74] pc: acpi: move remaining GPE handlers into SSDT List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: qemu-devel@nongnu.org On Tue, 22 Dec 2015 16:47:18 +0200 "Michael S. Tsirkin" wrote: > On Tue, Dec 22, 2015 at 03:38:24PM +0100, Igor Mammedov wrote: > > On Tue, 22 Dec 2015 11:37:40 +0200 > > "Michael S. Tsirkin" 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 > > Just add ... at the end: > > aml_method("_L0%X", argcount, AML_NOTSERIALIZED, i); ok, I'll try to do it. > > looks a bit weird if you have to use it, but > it's uncommon, and the common case looks simple. > > > 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. > > So how about just open-coding this loop for now? > That's just about 10 lines of code, > not a big deal. sure > > As you say, will be gone after refactoring. >