lkml.org 
[lkml]   [2015]   [Jul]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] Define find_symbol_in_section_t as function type to simplify the code
On 07/15/15 at 07:22am, Rusty Russell wrote:
> Minfei Huang <mhuang@redhat.com> writes:
> > From: Minfei Huang <mnfhuang@gmail.com>
> >
> > 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 <mnfhuang@gmail.com>
> > ---
> > 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


\
 
 \ /
  Last update: 2015-07-15 04:41    [W:0.060 / U:0.068 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site