lkml.org 
[lkml]   [2008]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: + markers-dont-risk-null-deref-in-marker-checkpatch-fixes.patch added to -mm tree
On 28/02/2008, akpm@linux-foundation.org <akpm@linux-foundation.org> wrote:
>
> The patch titled
> markers-dont-risk-null-deref-in-marker-checkpatch-fixes
> has been added to the -mm tree. Its filename is
> markers-dont-risk-null-deref-in-marker-checkpatch-fixes.patch
>
<snip>
> Subject: markers-dont-risk-null-deref-in-marker-checkpatch-fixes
> From: Andrew Morton <akpm@linux-foundation.org>
>

Andrew, I very much do not agree with you and/or checkpatch here :

> WARNING: braces {} are not necessary for single statement blocks
> #34: FILE: kernel/marker.c:715:
> + if (!entry) {
> + goto end;
> + }
>
> total: 0 errors, 1 warnings, 29 lines checked
>
> ./patches/markers-dont-risk-null-deref-in-marker.patch has style problems, please review. If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.
>
> Please run checkpatch prior to sending patches
>
> Cc: Jesper Juhl <jesper.juhl@gmail.com>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@polymtl.ca>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> kernel/marker.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff -puN kernel/marker.c~markers-dont-risk-null-deref-in-marker-checkpatch-fixes kernel/marker.c
> --- a/kernel/marker.c~markers-dont-risk-null-deref-in-marker-checkpatch-fixes
> +++ a/kernel/marker.c
> @@ -702,9 +702,8 @@ int marker_probe_unregister(const char *
>
> mutex_lock(&markers_mutex);
> entry = get_marker(name);
> - if (!entry) {
> + if (!entry)
> goto end;
> - }
> if (entry->rcu_pending)
> rcu_barrier();
> old = marker_entry_remove_probe(entry, probe, probe_private);
> @@ -712,9 +711,8 @@ int marker_probe_unregister(const char *
> marker_update_probes(); /* may update entry */
> mutex_lock(&markers_mutex);
> entry = get_marker(name);
> - if (!entry) {
> + if (!entry)
> goto end;
> - }
> entry->oldptr = old;
> entry->rcu_pending = 1;
> /* write rcu_pending before calling the RCU callback */

While it is entirely true that the compiler has no need for the extra
{} they are very nice for human readers/editors of the code - and if
what's inside happens to be a macro, also potentially safer.

I left them in very much on purpose.

--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html


\
 
 \ /
  Last update: 2008-02-28 23:39    [W:0.030 / U:0.096 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site