Re: [PATCH 2/2] Add document about exported enum definitions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

We don't use typedefs in kmod and actually mixing this change in a
commit that says to be about adding doc is pretty bad.

> +                                               kmod_filter filter_type,
>                                                 const struct kmod_list *input,
>                                                 struct kmod_list **output)
>  {
> @@ -1147,10 +1174,31 @@ static int kmod_module_get_probe_list(struct kmod_module *mod,
>  }
>
>  /**
> + * 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

ditto.

> + * @KMOD_PROBE_IGNORE_COMMAND: whether the module's command and softdep should
> + * be ignored

whether the probe should ignore install commands and softdeps
configured in the system

> + * @KMOD_PROBE_IGNORE_LOADED: do not check whether the module is already live in
> + * kernel or not
> + * @KMOD_PROBE_DRY_RUN: dry run, do not insert module

@KMOD_PROBE_DRY_RUN: dry run, do not insert module, just call the
associated callback function

> + * @KMOD_PROBE_FAIL_ON_LOADED: if KMOD_PROBE_IGNORE_LOADED not specified and
> + * the module is already live in kernel, the function will fail if this flag
> + * specified

some verbs missing:

@KMOD_PROBE_FAIL_ON_LOADED: if KMOD_PROBE_IGNORE_LOADED is not
specified and the module is already live in kernel, the function will
fail if this flag is specified.

> + * @KMOD_PROBE_APPLY_BLACKLIST_ALL: probe will fail if the module is in
> + * blacklist

probe will apply KMOD_FILTER_BLACKLIST filter to this module and its
dependencies. This will most likely fail the probe, unless a
blacklisted module is already loaded.


> + * @KMOD_PROBE_APPLY_BLACKLIST: probe will fail if the module is in blacklist

ok

> + * @KMOD_PROBE_APPLY_BLACKLIST_ALIAS_ONLY: probe will fail if the module is an
> + * alias and in blacklist

and is blacklisted

> + *
> + * kmod module probe flags used by kmod_module_probe_insert_module().
> + */
> +
> +/**
>   * kmod_module_probe_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_probe_flags.
>   * @extra_options: module's options to pass to Linux Kernel. It applies only
>   * to @mod, not to its dependencies.
>   * @run_install: function to run when @mod is backed by an install command.
> @@ -1619,15 +1667,28 @@ KMOD_EXPORT int kmod_module_new_from_loaded(struct kmod_ctx *ctx,
>  }
>
>  /**
> + * kmod_module_initstate:
> + * @KMOD_MODULE_BUILTIN: module is builtin
> + * @KMOD_MODULE_LIVE: module is live in kernel
> + * @KMOD_MODULE_COMING: module is being loaded
> + * @KMOD_MODULE_GOING: module is being unloaded
> + * @_KMOD_MODULE_PAD: do not use it
> + *
> + * kmod module init state, used by kmod_module_get_initstate()
> + * and kmod_module_initstate_str().
> + */
> +
> +/**
>   * kmod_module_initstate_str:
> - * @state: the state as returned by kmod_module_get_initstate()
> + * @state: the state as returned by kmod_module_get_initstate(), see
> + * #kmod_module_initstate.
>   *
>   * Translate a initstate to a string.
>   *
>   * Returns: the string associated to the @state. This string is statically
>   * allocated, do not free it.
>   */
> -KMOD_EXPORT const char *kmod_module_initstate_str(enum kmod_module_initstate state)
> +KMOD_EXPORT const char *kmod_module_initstate_str(kmod_module_initstate state)
>  {
>         switch (state) {
>         case KMOD_MODULE_BUILTIN:
> @@ -1650,7 +1711,7 @@ KMOD_EXPORT const char *kmod_module_initstate_str(enum kmod_module_initstate sta
>   * Get the initstate of this @mod, as returned by Linux Kernel, by reading
>   * /sys filesystem.
>   *
> - * Returns: < 0 on error or enum kmod_initstate if module is found in kernel.
> + * Returns: < 0 on error or #kmod_module_initstate if module is found in kernel.
>   */
>  KMOD_EXPORT int kmod_module_get_initstate(const struct kmod_module *mod)
>  {
> @@ -2378,7 +2439,7 @@ list_error:
>  }
>
>  /**
> - * kmod_module_versions_get_symbol:
> + * kmod_module_version_get_symbol:
>   * @entry: a list entry representing a kmod module versions
>   *
>   * Get the symbol of a kmod module versions.
> diff --git a/libkmod/libkmod.c b/libkmod/libkmod.c
> index 1113614..98cf15e 100644
> --- a/libkmod/libkmod.c
> +++ b/libkmod/libkmod.c
> @@ -48,6 +48,16 @@
>   * and is passed to all library operations.
>   */
>
> +/**
> + * kmod_resources_status:
> + * @KMOD_RESOURCES_OK: kmod resources are still valid
> + * @KMOD_RESOURCES_MUST_RELOAD: kmod resources are sufficient to call
> + * kmod_load_resources() and kmod_unload_resources()
> + * @KMOD_RESOURCES_MUST_RECREATE: kmod resources #kmod_ctx must be re-created.
> + *
> + * kmod resources status, see kmod_load_resources(), kmod_unload_resources()
> + * and kmod_validate_resources().
> + */
>  static struct _index_files {
>         const char *fn;
>         const char *prefix;
> @@ -740,12 +750,9 @@ static bool is_cache_invalid(const char *path, unsigned long long stamp)
>   * Check if indexes and configuration files changed on disk and the current
>   * context is not valid anymore.
>   *
> - * Returns: KMOD_RESOURCES_OK if resources are still valid,
> - * KMOD_RESOURCES_MUST_RELOAD if it's sufficient to call
> - * kmod_unload_resources() and kmod_load_resources() or
> - * KMOD_RESOURCES_MUST_RECREATE if @ctx must be re-created.
> + * Returns: kmod resources status, see #kmod_resources_status
>   */
> -KMOD_EXPORT int kmod_validate_resources(struct kmod_ctx *ctx)
> +KMOD_EXPORT kmod_resources_status kmod_validate_resources(struct kmod_ctx *ctx)

we can't change the API right now, so keep it as int, but you can
document it like you did.

>  {
>         struct kmod_list *l;
>         size_t i;
> diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
> index 3397f87..c1bec07 100644
> --- a/libkmod/libkmod.h
> +++ b/libkmod/libkmod.h
> @@ -59,13 +59,15 @@ void kmod_set_userdata(struct kmod_ctx *ctx, const void *userdata);
>  int kmod_load_resources(struct kmod_ctx *ctx);
>  void kmod_unload_resources(struct kmod_ctx *ctx);
>
> -enum kmod_resources {
> +/* kmod resources status */
> +typedef enum {
>         KMOD_RESOURCES_OK = 0,
>         KMOD_RESOURCES_MUST_RELOAD = 1,
>         KMOD_RESOURCES_MUST_RECREATE = 2,
> -};
> -int kmod_validate_resources(struct kmod_ctx *ctx);
> +} kmod_resources_status;
> +kmod_resources_status kmod_validate_resources(struct kmod_ctx *ctx);
>
> +/* kmod module type */
>  enum kmod_index {
>         KMOD_INDEX_MODULES_DEP = 0,
>         KMOD_INDEX_MODULES_ALIAS,
> @@ -138,19 +140,19 @@ struct kmod_module *kmod_module_get_module(const struct kmod_list *entry);
>
>
>  /* Removal flags */
> -enum kmod_remove {
> +typedef enum {
>         KMOD_REMOVE_FORCE = O_TRUNC,
>         KMOD_REMOVE_NOWAIT = O_NONBLOCK,
> -};
> +} kmod_module_remove_flags;
>
>  /* Insertion flags */
> -enum kmod_insert {
> +typedef enum {
>         KMOD_INSERT_FORCE_VERMAGIC = 0x1,
>         KMOD_INSERT_FORCE_MODVERSION = 0x2,
> -};
> +} kmod_module_insert_flags;
>
>  /* Flags to kmod_module_probe_insert_module() */
> -enum kmod_probe {
> +typedef enum {
>         KMOD_PROBE_FORCE_VERMAGIC =             0x00001,
>         KMOD_PROBE_FORCE_MODVERSION =           0x00002,
>         KMOD_PROBE_IGNORE_COMMAND =             0x00004,
> @@ -162,13 +164,13 @@ enum kmod_probe {
>         KMOD_PROBE_APPLY_BLACKLIST_ALL =        0x10000,
>         KMOD_PROBE_APPLY_BLACKLIST =            0x20000,
>         KMOD_PROBE_APPLY_BLACKLIST_ALIAS_ONLY = 0x40000,
> -};
> +} kmod_module_probe_flags;
>
>  /* Flags to kmod_module_apply_filter() */
> -enum kmod_filter {
> +typedef enum {
>         KMOD_FILTER_BLACKLIST = 0x00001,
>         KMOD_FILTER_BUILTIN = 0x00002,
> -};
> +} kmod_filter;
>
>  int kmod_module_remove_module(struct kmod_module *mod, unsigned int flags);
>  int kmod_module_insert_module(struct kmod_module *mod, unsigned int flags,
> @@ -194,7 +196,7 @@ int kmod_module_get_filtered_blacklist(const struct kmod_ctx *ctx,
>                                         const struct kmod_list *input,
>                                         struct kmod_list **output) __attribute__ ((deprecated));
>  int kmod_module_apply_filter(const struct kmod_ctx *ctx,
> -                                       enum kmod_filter filter_type,
> +                                       kmod_filter filter_type,
>                                         const struct kmod_list *input,
>                                         struct kmod_list **output);
>
> @@ -205,15 +207,15 @@ int kmod_module_apply_filter(const struct kmod_ctx *ctx,
>   * by kernel
>   */
>
> -enum kmod_module_initstate {
> +typedef enum {
>         KMOD_MODULE_BUILTIN = 0,
>         KMOD_MODULE_LIVE,
>         KMOD_MODULE_COMING,
>         KMOD_MODULE_GOING,
>         /* Padding to make sure enum is not mapped to char */
>         _KMOD_MODULE_PAD = (1 << 31),
> -};
> -const char *kmod_module_initstate_str(enum kmod_module_initstate state);
> +} kmod_module_initstate;
> +const char *kmod_module_initstate_str(kmod_module_initstate state);
>  int kmod_module_get_initstate(const struct kmod_module *mod);
>  int kmod_module_get_refcnt(const struct kmod_module *mod);
>  struct kmod_list *kmod_module_get_holders(const struct kmod_module *mod);
> --

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




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux