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;
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
