[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v9 5/8] module: implement module loading
From: |
Fam Zheng |
Subject: |
Re: [Qemu-devel] [PATCH v9 5/8] module: implement module loading |
Date: |
Mon, 16 Sep 2013 09:41:36 +0800 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, 09/13 07:52, Richard Henderson wrote:
> On 09/13/2013 02:59 AM, Fam Zheng wrote:
> > + const char *module_whitelist[] = {
> > + CONFIG_MODULE_WHITELIST
> > + };
>
> static const char * const module_whitelist[] = ...
>
OK, thanks.
> > + 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.
>
It's that I'd like to be consistent with block whitelist code style as in
bdrv_is_whitelisted().
> > + 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.
>
Perhaps using module type as prefix and building into whitelist can save us
creating subdirs for each type and also the separation of the list as you
suggested above:
switch (type) {
case MODULE_LOAD_BLOCK:
prefix = "block-";
break;
...
}
for (mp = &module_whitelist[0]; *mp; mp++) {
if (!strncmp(prefix, *mp, sizeof(prefix))) {
/* load */
}
}
- [Qemu-devel] [PATCH v9 2/8] make.rule: fix $(obj) to a real relative path, (continued)
- [Qemu-devel] [PATCH v9 2/8] make.rule: fix $(obj) to a real relative path, Fam Zheng, 2013/09/13
- [Qemu-devel] [PATCH v9 3/8] rule.mak: allow per object cflags and libs, Fam Zheng, 2013/09/13
- [Qemu-devel] [PATCH v9 4/8] build-sys: introduce common-obj-m and block-obj-m for DSO, Fam Zheng, 2013/09/13
- [Qemu-devel] [PATCH v9 6/8] Makefile: install modules with "make install", Fam Zheng, 2013/09/13
- [Qemu-devel] [PATCH v9 5/8] module: implement module loading, Fam Zheng, 2013/09/13
- [Qemu-devel] [PATCH v9 7/8] .gitignore: ignore module related files (dll, so, mo), Fam Zheng, 2013/09/13
- [Qemu-devel] [PATCH v9 8/8] block: convert block drivers linked with libs to modules, Fam Zheng, 2013/09/13
- Re: [Qemu-devel] [PATCH v9 0/8] Shared Library Module Support, Peter Maydell, 2013/09/13