From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58085) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKUjT-0005pi-8R for qemu-devel@nongnu.org; Fri, 13 Sep 2013 10:52:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VKUjK-0006wk-EV for qemu-devel@nongnu.org; Fri, 13 Sep 2013 10:52:27 -0400 Received: from mail-ob0-x231.google.com ([2607:f8b0:4003:c01::231]:45535) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VKUjK-0006vP-8X for qemu-devel@nongnu.org; Fri, 13 Sep 2013 10:52:18 -0400 Received: by mail-ob0-f177.google.com with SMTP id wp18so1166608obc.8 for ; Fri, 13 Sep 2013 07:52:17 -0700 (PDT) Sender: Richard Henderson Message-ID: <5233269B.3030803@twiddle.net> Date: Fri, 13 Sep 2013 07:52:11 -0700 From: Richard Henderson MIME-Version: 1.0 References: <1379066356-14986-1-git-send-email-famz@redhat.com> <1379066356-14986-6-git-send-email-famz@redhat.com> In-Reply-To: <1379066356-14986-6-git-send-email-famz@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v9 5/8] module: implement module loading List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: peter.maydell@linaro.org, mjt@tls.msk.ru, qemu-devel@nongnu.org, alex@alex.org.uk, pbonzini@redhat.com, vilanova@ac.upc.edu On 09/13/2013 02:59 AM, Fam Zheng wrote: > + const char *module_whitelist[] = { > + CONFIG_MODULE_WHITELIST > + }; static const char * const module_whitelist[] = ... > + switch (type) { > + case MODULE_LOAD_BLOCK: > + path = CONFIG_MODDIR "/block/"; > + break; > + case MODULE_LOAD_UI: > + path = CONFIG_MODDIR "/ui/"; > + break; > + case MODULE_LOAD_NET: > + path = CONFIG_MODDIR "/net/"; > + break; Also, separate the whitelists by type. I.e. static const char * const modules_block[] = ... static const char * const modules_ui[] = ... static const char * const modules_net[] = ... switch (type) { case MODULE_LOAD_BLOCK: list = modules_block; n = ARRAY_SIZE(modules_block); break; ... } No need for null termination of the array, as you're currently using. > + for (mp = &module_whitelist[0]; *mp; mp++) { > + fname = g_strdup_printf("%s%s" HOST_DSOSUF, path, *mp); > + module_load_file(fname); > + g_free(fname); > + } Why this bizzare mix of g_strdup_printf and compile-time string concatenation? Certainly you could have arranged for HOST_DSOSUF to be built into the module name as seen in the arrays. Then we're back to the subdirectory vs filename prefix and CONFIG_MODDIR vs any of several module search path options, the debate of which I don't believe has concluded. r~