Messages in this thread | | | Date | Mon, 05 Nov 2012 10:57:11 +1100 | From | Ryan Mallon <> | Subject | Re: [PATCH] Staging: Android: logger: module_exit implementation |
| |
On 02/11/12 17:15, Luca Clementi wrote: > Created the module_exit for the android logger so that > it can be loaded and unloaded as a module. Fixed > module_init and some other minor issues. > > Signed-off-by: Luca Clementi <luca.clementi@gmail.com> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Cc: Brian Swetland <swetland@google.com> > --- > drivers/staging/android/logger.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/android/logger.c b/drivers/staging/android/logger.c > index 1d5ed47..050be01 100644 > --- a/drivers/staging/android/logger.c > +++ b/drivers/staging/android/logger.c > @@ -676,4 +676,32 @@ static int __init logger_init(void) > out: > return ret; > } > -device_initcall(logger_init); > + > +static void __exit logger_exit(void) > +{ > + struct logger_log *current_log, *next_log; > + > + list_for_each_entry_safe(current_log, next_log, &log_list, logs) { > + /* we have to delete all the entry inside log_list */ > + ret = misc_deregister(¤t_log->misc);
ret is undeclared? Has this been tested?
> + if (unlikely(ret)) { > + pr_err("failed to deregister misc device for log '%s'!\n", > + current_log->misc.name); > + }
Don't need braces on single line if statements, and unlikely is not needed here (it isn't a hotpath). The whole error can probably just be removed since it looks like misc_deregister already does a WARN_ON if it fails.
> + pr_info("removed loggger '%s'\n", current_log->misc.name);
Don't spam the log with stuff like this. Either change to pr_debug, or probably just remove it.
> + vfree(current_log->buffer); > + kfree(current_log->misc.name); > + kfree(current_log);
Missing:
list_del(¤t_log->logs);
?
> + } > + > + return;
This is pointless.
> +} > + > + > +module_init(logger_init); > +module_exit(logger_exit);
Nitpick - Blank line here.
> +MODULE_LICENSE("GPL"); > +MODULE_AUTHOR("Brian Swetland, <swetland@google.com>");
Should be Robert Love according to the header comment.
> +MODULE_DESCRIPTION("Android Logger"); > + > +
Don't need extra blank lines at the end of the file.
~Ryan
| |