On Wed, Sep 24, 2014 at 11:56:19AM -0400, Benjamin Romer wrote:
> +struct periodic_work *
> + visor_periodic_work_create(ulong jiffy_interval,
> + struct workqueue_struct *workqueue,
> + void (*workfunc)(void *),
> + void *workfuncarg,
> + const char *devnam);
No. This isn't the right way to do it. The way the lines were broken
up originally was fine. It's ok to pull the parameter declarations back
to make it under the 80 character limit.
I wonder if we could do something like this in checkpatch.pl?
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 74bba23..79a7f82 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2503,7 +2503,8 @@ sub process {
my $goodspaceindent = $oldindent . " " x $pos;
if ($newindent ne $goodtabindent &&
- $newindent ne $goodspaceindent) {
+ $newindent ne $goodspaceindent &&
+ length($goodspaceindent) < 40) {
if (CHK("PARENTHESIS_ALIGNMENT",
"Alignment should match open parenthesis\n" . $hereprev) &&
> static void periodic_work_func(struct work_struct *work)
> {
> - PERIODIC_WORK *periodic_work =
> - container_of(work, struct PERIODIC_WORK_Tag, work.work);
> - (*periodic_work->workfunc)(periodic_work->workfuncarg);
> -}
> -
> + struct periodic_work *pw =
> + container_of(work, struct periodic_work, work.work);
>
> + (*pw->workfunc)(pw->workfuncarg);
> +}
Just do:
{
struct periodic_work *pw;
pw = container_of(work, struct periodic_work, work.work);
(*pw->workfunc)(pw->workfuncarg);
}
> + struct periodic_work *pw = kzalloc(sizeof(struct periodic_work),
> + GFP_KERNEL | __GFP_NORETRY);
> +
> + if (pw == NULL) {
Don't put a blank line between the call and the check.
Just say "if (!pw)" because it's more concise.
> ERRDRV("periodic_work allocation failed ");
Don't print an error here. kmalloc() has its own better printks.
> -void visor_periodic_work_destroy(PERIODIC_WORK *periodic_work)
> +void visor_periodic_work_destroy(struct periodic_work *pw)
> {
> - if (periodic_work == NULL)
> + if (pw == NULL)
> return;
> - kfree(periodic_work);
> + kfree(pw);
Don't check for NULL here. kfree() accepts NULLs.
> -Away:
> - write_unlock(&periodic_work->lock);
> +away:
> + write_unlock(&pw->lock);
unlock: is a better label than "away".
regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
