Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

[trivial PATCH] module.h: Remove unnecessary semicolon

0 views
Skip to first unread message

Joe Perches

unread,
Nov 5, 2013, 4:00:02 PM11/5/13
to
This semicolon isn't necessary, remove it.

Signed-off-by: Joe Perches <j...@perches.com>
---
include/linux/module.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 05f2447..d1ad477 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -454,7 +454,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

extern void __module_put_and_exit(struct module *mod, long code)
__attribute__((noreturn));
-#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code);
+#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)

#ifdef CONFIG_MODULE_UNLOAD
unsigned long module_refcount(struct module *mod);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Rusty Russell

unread,
Nov 6, 2013, 10:20:02 PM11/6/13
to
Joe Perches <j...@perches.com> writes:
> This semicolon isn't necessary, remove it.
>
> Signed-off-by: Joe Perches <j...@perches.com>

This is a terrible description. Really bad.

First, it just repeats the subject, with more words.

Second, it gives me no indication that you've done a grep to make sure
noone is abusing the macro, so I can't apply it without doing that check
myself.

Please try again.

Rusty.

Joe Perches

unread,
Nov 7, 2013, 12:20:01 AM11/7/13
to
On Thu, 2013-11-07 at 12:32 +1030, Rusty Russell wrote:
> Joe Perches <j...@perches.com> writes:
> > This semicolon isn't necessary, remove it.
> >
> > Signed-off-by: Joe Perches <j...@perches.com>
>
> This is a terrible description. Really bad.

I'd've preferred no description.

> First, it just repeats the subject, with more words.

Which others have demanded.

> Second, it gives me no indication that you've done a grep to make sure
> noone is abusing the macro, so I can't apply it without doing that check
> myself.

That's a trust issue.
I've done it. It isn't necessary.
The other #define module_put_and_exit in a
different #if #else already doesn't have one.

Trust it or not, apply it or not.

> Please try again.

No thanks.

cheers, Joe

> > diff --git a/include/linux/module.h b/include/linux/module.h
[]
> > @@ -454,7 +454,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,
> >
> > extern void __module_put_and_exit(struct module *mod, long code)
> > __attribute__((noreturn));
> > -#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code);
> > +#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)
> >
> > #ifdef CONFIG_MODULE_UNLOAD
> > unsigned long module_refcount(struct module *mod);

$ git grep -w module_put_and_exit
crypto/algboss.c: module_put_and_exit(0);
crypto/algboss.c: module_put_and_exit(0);
fs/cifs/connect.c: module_put_and_exit(0);
fs/nfs/nfs4state.c: module_put_and_exit(0);
fs/nfsd/nfssvc.c: module_put_and_exit(0);
include/linux/module.h:#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code);
include/linux/module.h:#define module_put_and_exit(code) do_exit(code)
net/bluetooth/bnep/core.c: module_put_and_exit(0);
net/bluetooth/cmtp/core.c: module_put_and_exit(0);
net/bluetooth/hidp/core.c: module_put_and_exit(0);

Rusty Russell

unread,
Nov 7, 2013, 7:00:02 PM11/7/13
to
Joe Perches <j...@perches.com> writes:
> On Thu, 2013-11-07 at 12:32 +1030, Rusty Russell wrote:
>> Joe Perches <j...@perches.com> writes:
>> > This semicolon isn't necessary, remove it.
>> >
>> > Signed-off-by: Joe Perches <j...@perches.com>
>>
>> This is a terrible description. Really bad.
>
> I'd've preferred no description.

Me too.

>> First, it just repeats the subject, with more words.
>
> Which others have demanded.

They're wrong.

>> Second, it gives me no indication that you've done a grep to make sure
>> noone is abusing the macro, so I can't apply it without doing that check
>> myself.
>
> That's a trust issue.
> I've done it. It isn't necessary.

WTF? Now you just said it's not necessary, I *know* I can't trust you.

> The other #define module_put_and_exit in a
> different #if #else already doesn't have one.

You didn't mention this, and even if you did, it's not sufficient. Some
code only ever gets compiled as a module, so it'd never hit the
!CONFIG_MODULES case.

> Trust it or not, apply it or not.

Now I know when I receive a patch from you I have to check it carefully,
because you can't be bothered.

I've fixed your patch, you can find it below.

From: Joe Perches <j...@perches.com>
Subject: module.h: Remove unnecessary semicolon

[All 8 callers already have semicolons. -- RR]

Signed-off-by: Joe Perches <j...@perches.com>
Signed-off-by: Rusty Russell <ru...@rustcorp.com.au>
---
include/linux/module.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 15cd6b1b211e..46e548fd502a 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -451,7 +451,7 @@ int module_kallsyms_on_each_symbol(int (*fn)(void *, const char *,

extern void __module_put_and_exit(struct module *mod, long code)
__attribute__((noreturn));
-#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code);
+#define module_put_and_exit(code) __module_put_and_exit(THIS_MODULE, code)

#ifdef CONFIG_MODULE_UNLOAD
unsigned long module_refcount(struct module *mod);

Joe Perches

unread,
Nov 7, 2013, 7:00:03 PM11/7/13
to
On Fri, 2013-11-08 at 09:41 +1030, Rusty Russell wrote:
> Joe Perches <j...@perches.com> writes:
> > On Thu, 2013-11-07 at 12:32 +1030, Rusty Russell wrote:
> >> Joe Perches <j...@perches.com> writes:
> >> > This semicolon isn't necessary, remove it.
> >> >
> >> > Signed-off-by: Joe Perches <j...@perches.com>
> >>
> >> This is a terrible description. Really bad.
> >
> > I'd've preferred no description.
>
> Me too.
>
> >> First, it just repeats the subject, with more words.
> >
> > Which others have demanded.
>
> They're wrong.

shrug. Can't please everyone.

> >> Second, it gives me no indication that you've done a grep to make sure
> >> noone is abusing the macro, so I can't apply it without doing that check
> >> myself.
> >
> > That's a trust issue.
> > I've done it. It isn't necessary.
>
> WTF? Now you just said it's not necessary, I *know* I can't trust you.

"It" in this case is the grep that I did
prior to sending the patch.

> > The other #define module_put_and_exit in a
> > different #if #else already doesn't have one.
>
> You didn't mention this, and even if you did, it's not sufficient. Some
> code only ever gets compiled as a module, so it'd never hit the
> !CONFIG_MODULES case.

Which would only show there was a latent bug.

> > Trust it or not, apply it or not.
>
> Now I know when I receive a patch from you I have to check it carefully,
> because you can't be bothered.

Sure. If you think so.
Nonetheless, I told you I'd checked it.

> I've fixed your patch, you can find it below.

No, not really fixed. You modified the commit log to
suit your taste.

cheers, Joe

Rusty Russell

unread,
Nov 7, 2013, 8:30:01 PM11/7/13
to
Joe Perches <j...@perches.com> writes:
> On Fri, 2013-11-08 at 09:41 +1030, Rusty Russell wrote:
>> Joe Perches <j...@perches.com> writes:
>> > That's a trust issue.
>> > I've done it. It isn't necessary.
>>
>> WTF? Now you just said it's not necessary, I *know* I can't trust you.
>
> "It" in this case is the grep that I did
> prior to sending the patch.

Hi Joe,

Apologies for my off tone. But I think we got tangled somewhere?

You said "It isn't necessary".

You maintain that grepping the source to find out if you'd broken
something "isn't necessary"?

git history shows me you've done lots of these cleanups. I'm pretty
sure that's not what you meant.

But as maintainer, it's annoying that I had to check myself. I don't
have personal experience in how diligent you are. If you'd just
mentioned it, it would have saved me a few minutes and streamlined my
workflow immensely.

Providing assurance makes me a happy maintainer. So I modified it
because it sets a clear example for others.

I hope that clarifies,
Rusty.

Joe Perches

unread,
Nov 7, 2013, 8:50:02 PM11/7/13
to
On Fri, 2013-11-08 at 11:56 +1030, Rusty Russell wrote:
> Joe Perches <j...@perches.com> writes:
> > On Fri, 2013-11-08 at 09:41 +1030, Rusty Russell wrote:
> >> Joe Perches <j...@perches.com> writes:
> >> > That's a trust issue.
> >> > I've done it. It isn't necessary.
> >>
> >> WTF? Now you just said it's not necessary, I *know* I can't trust you.
> >
> > "It" in this case is the grep that I did
> > prior to sending the patch.
>
> Hi Joe,

Hi Rusty.

> Apologies for my off tone. But I think we got tangled somewhere?
> You said "It isn't necessary".
>
> You maintain that grepping the source to find out if you'd broken
> something "isn't necessary"?
>
> git history shows me you've done lots of these cleanups. I'm pretty
> sure that's not what you meant.

Yeah, I think we did get a little crossed up there.

No, grep and compiling changes are fundamentals
before submitting trivial patches.

What I meant was I had done the grep (and compile
allyesconfig/allmodconfig of the files that use
module_put_and_exit) before sending the patch.

I don't think those are important details for a
commit message.

Dunno if you noticed that I attached the grep results
in my 1st reply.

Sometimes I do put details below the separator
--- line after the signed-off-by.

(I'll also admit I've sent patches without doing
the appropriate checks and screwed them up.
Lessons semi-learned)

> But as maintainer, it's annoying that I had to check myself. I don't
> have personal experience in how diligent you are. If you'd just
> mentioned it, it would have saved me a few minutes and streamlined my
> workflow immensely.
>
> Providing assurance makes me a happy maintainer. So I modified it
> because it sets a clear example for others.
>
> I hope that clarifies,
> Rusty.

No worries. Unfamiliarity also does have its own
set of issues. No problems on my side. Hope you
don't any any this way either.

cheers, Joe
0 new messages