On Sat, Apr 27, 2013 at 12:24 AM, Lucas De Marchi
<lucas.demarchi@xxxxxxxxxxxxxx> wrote:
> On Fri, Apr 26, 2013 at 11:38 PM, Yang Chengwei <chengwei.yang@xxxxxxxxx> wrote:
>> On Thu, Apr 25, 2013 at 01:19:16AM -0300, Lucas De Marchi wrote:
>>> On Wed, Apr 24, 2013 at 5:19 AM, Chengwei Yang <chengwei.yang@xxxxxxxxx> wrote:
>>> > There are several exported enum by libkmod without document, so this
>>> > patch mainly added documentation for below enums:
>>> > * kmod_resources_status
>>> > * kmod_module_insert_flags
>>> > * kmod_module_remove_flags
>>> > * kmod_module_probe_flags
>>> > * kmod_filter
>>> > * kmod_module_initstate
>>> >
>>> > Signed-off-by: Chengwei Yang <chengwei.yang@xxxxxxxxx>
>>> > ---
>>> > libkmod/docs/libkmod-sections.txt | 6 +++
>>> > libkmod/libkmod-module.c | 79 ++++++++++++++++++++++++++++++++-----
>>> > libkmod/libkmod.c | 17 +++++---
>>> > libkmod/libkmod.h | 32 ++++++++-------
>>> > 4 files changed, 105 insertions(+), 29 deletions(-)
>>> >
>>> > diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt
>>> > index e59ab7a..d826daf 100644
>>> > --- a/libkmod/docs/libkmod-sections.txt
>>> > +++ b/libkmod/docs/libkmod-sections.txt
>>> > @@ -5,6 +5,7 @@ kmod_new
>>> > kmod_ref
>>> > kmod_unref
>>> >
>>> > +kmod_resources_status
>>> > kmod_load_resources
>>> > kmod_unload_resources
>>> > kmod_validate_resources
>>> > @@ -53,13 +54,17 @@ kmod_module_ref
>>> > kmod_module_unref
>>> > kmod_module_unref_list
>>> >
>>> > +kmod_module_insert_flags
>>> > kmod_module_insert_module
>>> > +kmod_module_probe_flags
>>> > kmod_module_probe_insert_module
>>> > +kmod_module_remove_flags
>>> > kmod_module_remove_module
>>> >
>>> > kmod_module_get_module
>>> > kmod_module_get_dependencies
>>> > kmod_module_get_softdeps
>>> > +kmod_filter
>>> > kmod_module_apply_filter
>>> > kmod_module_get_filtered_blacklist
>>> > kmod_module_get_install_commands
>>> > @@ -98,6 +103,7 @@ kmod_module_info_get_value
>>> > <SECTION>
>>> > <FILE>libkmod-loaded</FILE>
>>> > kmod_module_new_from_loaded
>>> > +kmod_module_initstate
>>> > kmod_module_get_initstate
>>> > kmod_module_initstate_str
>>> > kmod_module_get_size
>>> > diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
>>> > index 4754eeb..6086379 100644
>>> > --- a/libkmod/libkmod-module.c
>>> > +++ b/libkmod/libkmod-module.c
>>> > @@ -581,6 +581,15 @@ KMOD_EXPORT int kmod_module_unref_list(struct kmod_list *list)
>>> > }
>>> >
>>> > /**
>>> > + * kmod_filter:
>>> > + * @KMOD_FILTER_BLACKLIST: filter modules in blacklist out
>>> > + * @KMOD_FILTER_BUILTIN: filter builtin modules out
>>> > + *
>>> > + * kmod module filter flags, used to filter modules, used by
>>> > + * kmod_module_apply_filter().
>>> > + */
>>> > +
>>> > +/**
>>> > * kmod_module_get_filtered_blacklist:
>>> > * @ctx: kmod library context
>>> > * @input: list of kmod_module to be filtered with blacklist
>>> > @@ -736,9 +745,18 @@ KMOD_EXPORT const char *kmod_module_get_path(const struct kmod_module *mod)
>>> > extern long delete_module(const char *name, unsigned int flags);
>>> >
>>> > /**
>>> > + * kmod_module_remove_flags:
>>> > + * @KMOD_REMOVE_FORCE: force remove module regardless its reference count
>>>
>>> Humn... this is ambiguous since user might think this is related to
>>> the kmod_module reference count, which it isn't. Please make it
>>> explicit like: "force module removal regardless if it's still in use
>>> by a kernel subsystem or other process"
>>>
>>> > + * @KMOD_REMOVE_NOWAIT: return immediately
>>>
>>> Actually we'd like to deprecate *not* using this flag so it might
>>> become the default in a future version. See rmmod, it's already
>>> warning the user that the "-w" flag is deprecated. Could you be a bit
>>> more verbose here?
>>>
>>> > + *
>>> > + * kmod module flags used by kmod_module_remove_module().
>>> > + */
>>> > +
>>> > +/**
>>> > * kmod_module_remove_module:
>>> > * @mod: kmod module
>>> > - * @flags: flags to pass to Linux kernel when removing the module
>>> > + * @flags: flags to pass to Linux kernel when removing the module,
>>> > + * see #kmod_module_remove_flags.
>>> > *
>>> > * Remove a module from Linux kernel.
>>> > *
>>> > @@ -767,10 +785,19 @@ KMOD_EXPORT int kmod_module_remove_module(struct kmod_module *mod,
>>> > extern long init_module(const void *mem, unsigned long len, const char *args);
>>> >
>>> > /**
>>> > + * kmod_module_insert_flags:
>>> > + * @KMOD_INSERT_FORCE_VERMAGIC: remove version magic to force insert module
>>> > + * @KMOD_INSERT_FORCE_MODVERSION: remove interface version to force insert module
>>> > + * insert module
>>>
>>>
>>> I think it's weird to call this "interface". Maybe do like is done in
>>> init_module man page:
>>>
>>> "Ignore symbol version hashes"
>>>
>>> > + *
>>> > + * kmod module flags used by kmod_module_insert_module().
>>> > + */
>>> > +
>>> > +/**
>>> > * kmod_module_insert_module:
>>> > * @mod: kmod module
>>> > * @flags: flags are not passed to Linux Kernel, but instead they dictate the
>>> > - * behavior of this function.
>>> > + * behavior of this function. See #kmod_module_insert_flags.
>>> > * @options: module's options to pass to Linux Kernel.
>>> > *
>>> > * Insert a module in Linux kernel. It opens the file pointed by @mod,
>>> > @@ -879,7 +906,7 @@ static bool module_is_blacklisted(struct kmod_module *mod)
>>> > /**
>>> > * kmod_module_apply_filter
>>> > * @ctx: kmod library context
>>> > - * @filter_type: bitmask to filter modules on
>>> > + * @filter_type: bitmask to filter modules out, see #kmod_filter.
>>> > * @input: list of kmod_module to be filtered
>>> > * @output: where to save the new list
>>> > *
>>> > @@ -890,7 +917,7 @@ static bool module_is_blacklisted(struct kmod_module *mod)
>>> > * list.
>>> > */
>>> > KMOD_EXPORT int kmod_module_apply_filter(const struct kmod_ctx *ctx,
>>> > - enum kmod_filter filter_type,
>>>
>>> Is this a limitation in gtk-doc that it can't generate the doc if it's
>>> not typedef'ed? I don't think so...
>>
>> I feel sorry to say that's true from my understand. It recognize
>> typedef'ed enum and enum _foo {...}; In the later the document generated
>> also say enum foo (no underscore), it's wrong if there is no typedef
>> enum _foo foo since in fact no enum foo here.
>>
>> For detail, please refer to https://bugzilla.gnome.org/show_bug.cgi?id=657444
>>
>> So due to this limitation, the workaround I could say is use typedef as
>> below:
>>
>> typedef enum foo {
>> ...
>> } foo;
>>
>> Is that acceptable to you? If no, I think I'll drop the second patch
>> which try to document the exported enums.
>
> It's not acceptable in the public header and throughout the source
> code. Since this is a limitation of gtk-doc and that in C you can add
> the typedef anywhere, what could be done is to document these enums
> hidden in the end of the respective .c file. And throughout the source
> code we continue to use "enum ...". So you wouldn't touch the header,
> but would have this, for example in the ende of
> libkmod/libkmod-module.c:
>
>
> /** kmod_module_probe_flags:
> * @KMOD_PROBE_FORCE_VERMAGIC: remove version magic to force insert module
> * @KMOD_PROBE_FORCE_MODVERSION: remove interface version to force insert module
> * ...
> */
> typedef enum kmod_module_probe_flags kmod_module_probe_flags;
oh... and don't rename the enums, otherwise we break the API. This one
should be "enum kmod_probe", not kmod_module_probe_flags.
Lucas De Marchi
--
To unsubscribe from this list: send the line "unsubscribe linux-modules" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
