From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40643) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBO5A-0004Ln-GN for qemu-devel@nongnu.org; Tue, 22 Dec 2015 09:38:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aBO56-0006Cj-GH for qemu-devel@nongnu.org; Tue, 22 Dec 2015 09:38:32 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59874) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aBO56-0006Cd-9z for qemu-devel@nongnu.org; Tue, 22 Dec 2015 09:38:28 -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 B62CC8E395 for ; Tue, 22 Dec 2015 14:38:27 +0000 (UTC) Date: Tue, 22 Dec 2015 15:38:24 +0100 From: Igor Mammedov Message-ID: <20151222153824.13dcff98@nial.brq.redhat.com> In-Reply-To: <20151222113548-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> 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 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 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.