From: Minfei Huang <[email protected]>
This patchset do the cleanup. For now, we can use function type
as the parament to simplify the code.
Previously, we will declare the function as following:
bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
struct module *owner,
void *data), void *data);
Once we define the function as a type, we can just declare the function
as following.
bool each_symbol_section(find_symbol_in_section_t fn, void *data);
Minfei Huang (2):
Define find_symbol_in_section_t as function type to simplify the code
Define kallsyms_cmp_symbol_t as function type to simplify the code
include/linux/kallsyms.h | 10 +++-------
include/linux/module.h | 19 +++++++++----------
kernel/kallsyms.c | 4 +---
kernel/module.c | 13 +++----------
4 files changed, 16 insertions(+), 30 deletions(-)
--
2.2.2
From: Minfei Huang <[email protected]>
It is not elegance, if we use function directly as the argument, like
following:
bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
struct module *owner,
void *data), void *data);
Here introduce a type defined function find_symbol_in_section_t. Now
we can use these type defined function directly, if we want to pass
the function as the argument.
bool each_symbol_section(find_symbol_in_section_t fn, void *data);
Signed-off-by: Minfei Huang <[email protected]>
---
include/linux/module.h | 6 +++---
kernel/module.c | 9 ++-------
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index d67b193..1e125b1 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -462,14 +462,14 @@ const struct kernel_symbol *find_symbol(const char *name,
bool gplok,
bool warn);
+typedef bool (*find_symbol_in_section_t)(const struct symsearch *arr,
+ struct module *owner, void *data);
/*
* Walk the exported symbol table
*
* Must be called with module_mutex held or preemption disabled.
*/
-bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
- struct module *owner,
- void *data), void *data);
+bool each_symbol_section(find_symbol_in_section_t fn, void *data);
/* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
symnum out of range. */
diff --git a/kernel/module.c b/kernel/module.c
index 4d2b82e..1400c0b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -426,9 +426,7 @@ extern const unsigned long __start___kcrctab_unused_gpl[];
static bool each_symbol_in_section(const struct symsearch *arr,
unsigned int arrsize,
struct module *owner,
- bool (*fn)(const struct symsearch *syms,
- struct module *owner,
- void *data),
+ find_symbol_in_section_t fn,
void *data)
{
unsigned int j;
@@ -442,10 +440,7 @@ static bool each_symbol_in_section(const struct symsearch *arr,
}
/* Returns true as soon as fn returns true, otherwise false. */
-bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
- struct module *owner,
- void *data),
- void *data)
+bool each_symbol_section(find_symbol_in_section_t fn, void *data)
{
struct module *mod;
static const struct symsearch arr[] = {
--
2.2.2
From: Minfei Huang <[email protected]>
It is not elegance, if we use function directly as the argument, like
following:
int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
struct module *, unsigned long),
void *data);
Here introduce a type defined function kallsyms_cmp_symbol_t. Now
we can use these type defined function directly, if we want to pass
the function as the argument.
int module_kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn,
void *data);
Signed-off-by: Minfei Huang <[email protected]>
---
include/linux/kallsyms.h | 10 +++-------
include/linux/module.h | 13 ++++++-------
kernel/kallsyms.c | 4 +---
kernel/module.c | 4 +---
4 files changed, 11 insertions(+), 20 deletions(-)
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index 6883e19..e8ed37d 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -8,6 +8,7 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/stddef.h>
+#include <linux/module.h>
#define KSYM_NAME_LEN 128
#define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s]") + (KSYM_NAME_LEN - 1) + \
@@ -20,9 +21,7 @@ struct module;
unsigned long kallsyms_lookup_name(const char *name);
/* Call a function on each kallsyms symbol in the core kernel */
-int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
- unsigned long),
- void *data);
+int kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data);
extern int kallsyms_lookup_size_offset(unsigned long addr,
unsigned long *symbolsize,
@@ -52,10 +51,7 @@ static inline unsigned long kallsyms_lookup_name(const char *name)
return 0;
}
-static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *,
- unsigned long),
- void *data)
+static inline int kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data)
{
return 0;
}
diff --git a/include/linux/module.h b/include/linux/module.h
index 1e125b1..6a05a24 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -479,9 +479,10 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
/* Look for this name: can be of form module:name. */
unsigned long module_kallsyms_lookup_name(const char *name);
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *, unsigned long),
- void *data);
+typedef int (*kallsyms_cmp_symbol_t)(void *, const char *,
+ struct module *, unsigned long);
+
+int module_kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data);
extern void __module_put_and_exit(struct module *mod, long code)
__attribute__((noreturn));
@@ -637,10 +638,8 @@ static inline unsigned long module_kallsyms_lookup_name(const char *name)
return 0;
}
-static inline int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *,
- unsigned long),
- void *data)
+static inline int module_kallsyms_on_each_symbol(
+ kallsyms_cmp_symbol_t fn, void *data)
{
return 0;
}
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 5c5987f..be5786b 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -193,9 +193,7 @@ unsigned long kallsyms_lookup_name(const char *name)
}
EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
-int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
- unsigned long),
- void *data)
+int kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data)
{
char namebuf[KSYM_NAME_LEN];
unsigned long i;
diff --git a/kernel/module.c b/kernel/module.c
index 1400c0b..67ed39b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3811,9 +3811,7 @@ unsigned long module_kallsyms_lookup_name(const char *name)
return ret;
}
-int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
- struct module *, unsigned long),
- void *data)
+int module_kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data)
{
struct module *mod;
unsigned int i;
--
2.2.2
Lost the character 'n' in the Namhyung Kim <[email protected]>.
Resend it.
On 07/14/15 at 02:59P, Minfei Huang wrote:
> From: Minfei Huang <[email protected]>
>
> It is not elegance, if we use function directly as the argument, like
> following:
>
> bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> struct module *owner,
> void *data), void *data);
>
> Here introduce a type defined function find_symbol_in_section_t. Now
> we can use these type defined function directly, if we want to pass
> the function as the argument.
>
> bool each_symbol_section(find_symbol_in_section_t fn, void *data);
>
> Signed-off-by: Minfei Huang <[email protected]>
> ---
> include/linux/module.h | 6 +++---
> kernel/module.c | 9 ++-------
> 2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index d67b193..1e125b1 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -462,14 +462,14 @@ const struct kernel_symbol *find_symbol(const char *name,
> bool gplok,
> bool warn);
>
> +typedef bool (*find_symbol_in_section_t)(const struct symsearch *arr,
> + struct module *owner, void *data);
> /*
> * Walk the exported symbol table
> *
> * Must be called with module_mutex held or preemption disabled.
> */
> -bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> - struct module *owner,
> - void *data), void *data);
> +bool each_symbol_section(find_symbol_in_section_t fn, void *data);
>
> /* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
> symnum out of range. */
> diff --git a/kernel/module.c b/kernel/module.c
> index 4d2b82e..1400c0b 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -426,9 +426,7 @@ extern const unsigned long __start___kcrctab_unused_gpl[];
> static bool each_symbol_in_section(const struct symsearch *arr,
> unsigned int arrsize,
> struct module *owner,
> - bool (*fn)(const struct symsearch *syms,
> - struct module *owner,
> - void *data),
> + find_symbol_in_section_t fn,
> void *data)
> {
> unsigned int j;
> @@ -442,10 +440,7 @@ static bool each_symbol_in_section(const struct symsearch *arr,
> }
>
> /* Returns true as soon as fn returns true, otherwise false. */
> -bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> - struct module *owner,
> - void *data),
> - void *data)
> +bool each_symbol_section(find_symbol_in_section_t fn, void *data)
> {
> struct module *mod;
> static const struct symsearch arr[] = {
> --
> 2.2.2
>
Lost the character 'n' in the Namhyung Kim <[email protected]>.
Resent it.
On 07/14/15 at 02:59P, Minfei Huang wrote:
> From: Minfei Huang <[email protected]>
>
> It is not elegance, if we use function directly as the argument, like
> following:
>
> int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> struct module *, unsigned long),
> void *data);
>
> Here introduce a type defined function kallsyms_cmp_symbol_t. Now
> we can use these type defined function directly, if we want to pass
> the function as the argument.
>
> int module_kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn,
> void *data);
>
> Signed-off-by: Minfei Huang <[email protected]>
> ---
> include/linux/kallsyms.h | 10 +++-------
> include/linux/module.h | 13 ++++++-------
> kernel/kallsyms.c | 4 +---
> kernel/module.c | 4 +---
> 4 files changed, 11 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
> index 6883e19..e8ed37d 100644
> --- a/include/linux/kallsyms.h
> +++ b/include/linux/kallsyms.h
> @@ -8,6 +8,7 @@
> #include <linux/errno.h>
> #include <linux/kernel.h>
> #include <linux/stddef.h>
> +#include <linux/module.h>
>
> #define KSYM_NAME_LEN 128
> #define KSYM_SYMBOL_LEN (sizeof("%s+%#lx/%#lx [%s]") + (KSYM_NAME_LEN - 1) + \
> @@ -20,9 +21,7 @@ struct module;
> unsigned long kallsyms_lookup_name(const char *name);
>
> /* Call a function on each kallsyms symbol in the core kernel */
> -int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
> - unsigned long),
> - void *data);
> +int kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data);
>
> extern int kallsyms_lookup_size_offset(unsigned long addr,
> unsigned long *symbolsize,
> @@ -52,10 +51,7 @@ static inline unsigned long kallsyms_lookup_name(const char *name)
> return 0;
> }
>
> -static inline int kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> - struct module *,
> - unsigned long),
> - void *data)
> +static inline int kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data)
> {
> return 0;
> }
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 1e125b1..6a05a24 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -479,9 +479,10 @@ int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
> /* Look for this name: can be of form module:name. */
> unsigned long module_kallsyms_lookup_name(const char *name);
>
> -int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> - struct module *, unsigned long),
> - void *data);
> +typedef int (*kallsyms_cmp_symbol_t)(void *, const char *,
> + struct module *, unsigned long);
> +
> +int module_kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data);
>
> extern void __module_put_and_exit(struct module *mod, long code)
> __attribute__((noreturn));
> @@ -637,10 +638,8 @@ static inline unsigned long module_kallsyms_lookup_name(const char *name)
> return 0;
> }
>
> -static inline int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> - struct module *,
> - unsigned long),
> - void *data)
> +static inline int module_kallsyms_on_each_symbol(
> + kallsyms_cmp_symbol_t fn, void *data)
> {
> return 0;
> }
> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> index 5c5987f..be5786b 100644
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c
> @@ -193,9 +193,7 @@ unsigned long kallsyms_lookup_name(const char *name)
> }
> EXPORT_SYMBOL_GPL(kallsyms_lookup_name);
>
> -int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
> - unsigned long),
> - void *data)
> +int kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data)
> {
> char namebuf[KSYM_NAME_LEN];
> unsigned long i;
> diff --git a/kernel/module.c b/kernel/module.c
> index 1400c0b..67ed39b 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -3811,9 +3811,7 @@ unsigned long module_kallsyms_lookup_name(const char *name)
> return ret;
> }
>
> -int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> - struct module *, unsigned long),
> - void *data)
> +int module_kallsyms_on_each_symbol(kallsyms_cmp_symbol_t fn, void *data)
> {
> struct module *mod;
> unsigned int i;
> --
> 2.2.2
>
Lost the character 'n' in the Namhyung Kim <[email protected]>.
Resent it.
On 07/14/15 at 02:59P, Minfei Huang wrote:
> From: Minfei Huang <[email protected]>
>
> This patchset do the cleanup. For now, we can use function type
> as the parament to simplify the code.
>
> Previously, we will declare the function as following:
>
> bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> struct module *owner,
> void *data), void *data);
>
> Once we define the function as a type, we can just declare the function
> as following.
>
> bool each_symbol_section(find_symbol_in_section_t fn, void *data);
>
> Minfei Huang (2):
> Define find_symbol_in_section_t as function type to simplify the code
> Define kallsyms_cmp_symbol_t as function type to simplify the code
>
> include/linux/kallsyms.h | 10 +++-------
> include/linux/module.h | 19 +++++++++----------
> kernel/kallsyms.c | 4 +---
> kernel/module.c | 13 +++----------
> 4 files changed, 16 insertions(+), 30 deletions(-)
>
> --
> 2.2.2
>
Minfei Huang <[email protected]> writes:
> From: Minfei Huang <[email protected]>
>
> It is not elegance, if we use function directly as the argument, like
> following:
>
> bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> struct module *owner,
> void *data), void *data);
>
> Here introduce a type defined function find_symbol_in_section_t. Now
> we can use these type defined function directly, if we want to pass
> the function as the argument.
>
> bool each_symbol_section(find_symbol_in_section_t fn, void *data);
I disagree.
It's shorter, but it's less clear. typedefs on functions are not very
useful:
1) They require readers to look in two places to see how to use the
function (ie each_symbol_section).
2) They can't use the typedef to declare their function, since that
doesn't work in C.
If the function were being used many times, it makes sense. But
it's only used twice, once static inside module.c.
So I won't be applying these.
Cheers,
Rusty.
> Signed-off-by: Minfei Huang <[email protected]>
> ---
> include/linux/module.h | 6 +++---
> kernel/module.c | 9 ++-------
> 2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index d67b193..1e125b1 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -462,14 +462,14 @@ const struct kernel_symbol *find_symbol(const char *name,
> bool gplok,
> bool warn);
>
> +typedef bool (*find_symbol_in_section_t)(const struct symsearch *arr,
> + struct module *owner, void *data);
> /*
> * Walk the exported symbol table
> *
> * Must be called with module_mutex held or preemption disabled.
> */
> -bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> - struct module *owner,
> - void *data), void *data);
> +bool each_symbol_section(find_symbol_in_section_t fn, void *data);
>
> /* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
> symnum out of range. */
> diff --git a/kernel/module.c b/kernel/module.c
> index 4d2b82e..1400c0b 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -426,9 +426,7 @@ extern const unsigned long __start___kcrctab_unused_gpl[];
> static bool each_symbol_in_section(const struct symsearch *arr,
> unsigned int arrsize,
> struct module *owner,
> - bool (*fn)(const struct symsearch *syms,
> - struct module *owner,
> - void *data),
> + find_symbol_in_section_t fn,
> void *data)
> {
> unsigned int j;
> @@ -442,10 +440,7 @@ static bool each_symbol_in_section(const struct symsearch *arr,
> }
>
> /* Returns true as soon as fn returns true, otherwise false. */
> -bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> - struct module *owner,
> - void *data),
> - void *data)
> +bool each_symbol_section(find_symbol_in_section_t fn, void *data)
> {
> struct module *mod;
> static const struct symsearch arr[] = {
> --
> 2.2.2
On 07/15/15 at 07:22am, Rusty Russell wrote:
> Minfei Huang <[email protected]> writes:
> > From: Minfei Huang <[email protected]>
> >
> > It is not elegance, if we use function directly as the argument, like
> > following:
> >
> > bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> > struct module *owner,
> > void *data), void *data);
> >
> > Here introduce a type defined function find_symbol_in_section_t. Now
> > we can use these type defined function directly, if we want to pass
> > the function as the argument.
> >
> > bool each_symbol_section(find_symbol_in_section_t fn, void *data);
>
> I disagree.
>
> It's shorter, but it's less clear. typedefs on functions are not very
> useful:
> 1) They require readers to look in two places to see how to use the
> function (ie each_symbol_section).
> 2) They can't use the typedef to declare their function, since that
> doesn't work in C.
>
> If the function were being used many times, it makes sense. But
> it's only used twice, once static inside module.c.
>
> So I won't be applying these.
>
> Cheers,
> Rusty.
>
Hi, Rusty.
The main reason why I posted these patch is the function has too much
parameters which may be confused with readers at the first glance.
So it is better define a typedef function, although the readers shall
dig out what find_symbol_in_section_t is.
Thanks
Minfei
> > Signed-off-by: Minfei Huang <[email protected]>
> > ---
> > include/linux/module.h | 6 +++---
> > kernel/module.c | 9 ++-------
> > 2 files changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index d67b193..1e125b1 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -462,14 +462,14 @@ const struct kernel_symbol *find_symbol(const char *name,
> > bool gplok,
> > bool warn);
> >
> > +typedef bool (*find_symbol_in_section_t)(const struct symsearch *arr,
> > + struct module *owner, void *data);
> > /*
> > * Walk the exported symbol table
> > *
> > * Must be called with module_mutex held or preemption disabled.
> > */
> > -bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> > - struct module *owner,
> > - void *data), void *data);
> > +bool each_symbol_section(find_symbol_in_section_t fn, void *data);
> >
> > /* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
> > symnum out of range. */
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 4d2b82e..1400c0b 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -426,9 +426,7 @@ extern const unsigned long __start___kcrctab_unused_gpl[];
> > static bool each_symbol_in_section(const struct symsearch *arr,
> > unsigned int arrsize,
> > struct module *owner,
> > - bool (*fn)(const struct symsearch *syms,
> > - struct module *owner,
> > - void *data),
> > + find_symbol_in_section_t fn,
> > void *data)
> > {
> > unsigned int j;
> > @@ -442,10 +440,7 @@ static bool each_symbol_in_section(const struct symsearch *arr,
> > }
> >
> > /* Returns true as soon as fn returns true, otherwise false. */
> > -bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> > - struct module *owner,
> > - void *data),
> > - void *data)
> > +bool each_symbol_section(find_symbol_in_section_t fn, void *data)
> > {
> > struct module *mod;
> > static const struct symsearch arr[] = {
> > --
> > 2.2.2
On Wed, 15 Jul 2015 07:22:32 +0930 Rusty Russell <[email protected]> wrote:
> Minfei Huang <[email protected]> writes:
> > From: Minfei Huang <[email protected]>
> >
> > It is not elegance, if we use function directly as the argument, like
> > following:
> >
> > bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
> > struct module *owner,
> > void *data), void *data);
> >
> > Here introduce a type defined function find_symbol_in_section_t. Now
> > we can use these type defined function directly, if we want to pass
> > the function as the argument.
> >
> > bool each_symbol_section(find_symbol_in_section_t fn, void *data);
>
> I disagree.
>
> It's shorter, but it's less clear. typedefs on functions are not very
> useful:
> 1) They require readers to look in two places to see how to use the
> function (ie each_symbol_section).
> 2) They can't use the typedef to declare their function, since that
> doesn't work in C.
>
> If the function were being used many times, it makes sense. But
> it's only used twice, once static inside module.c.
>
Using a foo_t typedef for a function callback is a common pattern.
It's (almost) the only approved use of typedefs. The usage is
widespread enough that when one sees a foo_t type, one says "ahah,
that's a function pointer".
Sorry, but I don't think "Rusty doesn't like it" is a good reason for
the module code to be different. All of us dislike some aspects of
kernel coding practices, but we go along because consistency is more
important.
Andrew Morton <[email protected]> writes:
> On Wed, 15 Jul 2015 07:22:32 +0930 Rusty Russell <[email protected]> wrote:
>> It's shorter, but it's less clear. typedefs on functions are not very
>> useful:
>> 1) They require readers to look in two places to see how to use the
>> function (ie each_symbol_section).
>> 2) They can't use the typedef to declare their function, since that
>> doesn't work in C.
>>
>> If the function were being used many times, it makes sense. But
>> it's only used twice, once static inside module.c.
>>
>
> Using a foo_t typedef for a function callback is a common pattern.
> It's (almost) the only approved use of typedefs. The usage is
> widespread enough that when one sees a foo_t type, one says "ahah,
> that's a function pointer".
I always thought of a type which can map to varying types under
different arch/configs as the typical typedef.
> Sorry, but I don't think "Rusty doesn't like it" is a good reason for
> the module code to be different.
But "Rusty has to maintain it" is a pretty strong counter argument,
IMHO.
> All of us dislike some aspects of
> kernel coding practices, but we go along because consistency is more
> important.
Consistency is important when it makes things more readable, sure.
I don't think any kernel devs are going to get confused seeing a
function pointer, and I think this patch makes the code slightly
less readable.
Enough not to apply the patch, but not enough waste more time on it.
Cheers,
Rusty.