[03/10] md/dm-crypt: Rename a jump label in crypt_message()

Commit Message

SF Markus Elfring Sept. 28, 2016, 3:40 p.m. UTC

From: Markus Elfring <[e-mail]>
Date: Wed, 28 Sep 2016 14:54:39 +0200

Adjust a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <[e-mail]>
---
 drivers/md/dm-crypt.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Comments

Theodore Ts'o Sept. 29, 2016, 12:55 p.m. UTC | #1

On Wed, Sep 28, 2016 at 05:40:14PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <[e-mail]>
> Date: Wed, 28 Sep 2016 14:54:39 +0200
> 
> Adjust a jump label according to the current Linux coding style convention.

In what bizzaro world is the "current Linux coding style convention"

> -
> -error:
> +show_warning:
>  	DMWARN("unrecognised message received.");
>  	return -EINVAL;
>  }

"show_warning" is better than "error" when the net result of the goto
is that the function returns -EINVAL?!?

Please give it up with these drive-by shooting of auto-generated
patches.  You're just embarassing yourself.

							- Ted

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

SF Markus Elfring Sept. 29, 2016, 3:43 p.m. UTC | #2

> In what bizzaro world is the "current Linux coding style convention"

Do you look at the evolution for a document like "CodingStyle"?


>> -
>> -error:
>> +show_warning:
>>  	DMWARN("unrecognised message received.");
>>  	return -EINVAL;
>>  }
> 
> "show_warning" is better than "error"

I got such an impression.


> when the net result of the goto is that the function returns -EINVAL?!?

Do other identifiers fit better for the desired description of "what" and "why"
by jump labels?


> Please give it up with these drive-by shooting of auto-generated patches.

This update step was not auto-generated.

There are further change possibilities where special analysis tools
can help in the corresponding software development.


> You're just embarassing yourself.

Do you find any of my update suggestions worth for further considerations?

Regards,
Markus

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Dan Carpenter Sept. 30, 2016, 10:06 a.m. UTC | #3

On Thu, Sep 29, 2016 at 05:43:57PM +0200, SF Markus Elfring wrote:
> > In what bizzaro world is the "current Linux coding style convention"
> 
> Do you look at the evolution for a document like "CodingStyle"?
> 

Again, I wrote the paragraph in CodingStyle.  I just said that it's a
good idea to think about label names instead of using GW-BASIC style
numbered labels, I didn't say go around bothering everyone with waste
of time cleanup patches.

I specifically did not say that "out:" or "error:" labels are bad names.
Those are common style in the kernel.

Please stop sending these patches.

regards,
dan carpenter

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

SF Markus Elfring Sept. 30, 2016, 11:32 a.m. UTC | #4

> Again, I wrote the paragraph in CodingStyle.

This is obvious from the corresponding commit "add some more error
handling guidelines" (ea04036032edda6f771c1381d03832d2ed0f6c31
on 2014-12-02).


> I just said that it's a good idea to think about label names

I agree also to such a desire.


> instead of using GW-BASIC style numbered labels,

Is this kind of wording another weakness in the discussed document?
How many guidance do programmers get from such a software specification?

I came a few source code places along where I proposed corresponding changes.


> I didn't say

You did not say anything about some details as it is often easier to express
several aspects in vague and general terms.


> go around bothering everyone with waste of time cleanup patches.

I find it still debatable if the shown software development efforts
are really "wasted".

It seems that also the Linux development community is mixed about
related interpretations.


> I specifically did not say that "out:" or "error:" labels are bad names.

Did you inform me once that you had also a special opinion about an identifier
like "out"?

The C compiler will accept them as usual. But do we occasionally prefer
to express implementation details a bit better there?


> Those are common style in the kernel.

* Which impressions can you get from a statement like "goto fail;"
  or "goto error;"?

* Do any exception handling implementations should be reconsidered
  at such places?


> Please stop sending these patches.

Could it happen that the change acceptance will increase also for
the suggested renaming of jump labels if maintainers from other subsystems
would dare to respond once more in a positive way for such a software refactoring?

Regards,
Markus

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Bjørn Mork Sept. 30, 2016, 11:39 a.m. UTC | #5

SF Markus Elfring <[e-mail]> writes:

>> go around bothering everyone with waste of time cleanup patches.
>
> I find it still debatable if the shown software development efforts
> are really "wasted".

When someone tells you that you are wasting their time, then that is not
a subject for further discussion.


Bjørn

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

SF Markus Elfring Sept. 30, 2016, 11:53 a.m. UTC | #6

> When someone tells you that you are wasting their time,

This information can be useful to some degree


> then that is not a subject for further discussion.

I got an other impression. I guess that there are constraints for such a response
which can become interesting for further considerations.

Is it just usual that other software changes are more welcome?

Regards,
Markus

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Dan Carpenter Sept. 30, 2016, 12:02 p.m. UTC | #7

On Fri, Sep 30, 2016 at 01:32:23PM +0200, SF Markus Elfring wrote:
> > I specifically did not say that "out:" or "error:" labels are bad names.
> 
> Did you inform me once that you had also a special opinion about an identifier
> like "out"?

I don't like out labels, but that's my opinion.  There is nothing in
CodingStyle which says you can't do it.

regards,
dan carpenter

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Bjørn Mork Sept. 30, 2016, 12:06 p.m. UTC | #8

SF Markus Elfring <[e-mail]> writes:

>> When someone tells you that you are wasting their time,
>
> This information can be useful to some degree

Yes.  If you continue discussing after that point, then you make a clear
statement that it isn't an accident.  You are deliberately wasting their
time.

A lot of people already know this.  But you're right that it would be
useful to make it even clearer.  Maybe you could add a note about it to
each patch?  Something along "I will not listen.  I will not change.
Nothing you tell me will ever make it worth your time to do so"?

Just an idea...



Bjørn

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

SF Markus Elfring Sept. 30, 2016, 12:19 p.m. UTC | #9

> I don't like out labels, but that's my opinion.

Thanks for this acknowledgement that you have still got a special opinion
about such an identifier.

 
> There is nothing in CodingStyle which says you can't do it.

Does the terse description there try to suggest also to choose
better identifiers for source code places?

Does the meaning of such a coding style specification include also
the selection of a more pleasing identifier than "error"
for this software module?

Regards,
Markus

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

SF Markus Elfring Sept. 30, 2016, 12:54 p.m. UTC | #10

> If you continue discussing after that point,

I guess that such a condition is not needed.


> then you make a clear statement that it isn't an accident.

I hope that most of my software development activities are not "an accident".
Is the intent for any update suggestion (like the renaming of a jump label
in this case) reasonable to some degree?


> You are deliberately wasting their time.

I imagine that I do not really try to "waste" others time. But I am trying
also to change some "things".
There are circumstances when these contributions are interpreted as "wasted efforts".
Is the change acceptance usually higher for other update patterns?


> Something along "I will not listen.

I am listening while my responses might not fit to your current expectations.


> I will not change.

I have got also some personal change opportunities.


> Nothing you tell me will ever make it worth your time to do so"?

While you can be so clear about a rejection for this software module at the moment,
other contributors showed occasionally more positive information.

Regards,
Markus

--
dm-devel mailing list
[e-mail]
https://www.redhat.com/mailman/listinfo/dm-devel

Patch

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 88a3b62..08e3de2 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2016,7 +2016,7 @@  static int crypt_message(struct dm_target *ti, unsigned argc, char **argv)
 	int ret = -EINVAL;
 
 	if (argc < 2)
-		goto error;
+		goto show_warning;
 
 	if (!strcasecmp(argv[0], "key")) {
 		if (!test_bit(DM_CRYPT_SUSPENDED, &cc->flags)) {
@@ -2040,8 +2040,7 @@  static int crypt_message(struct dm_target *ti, unsigned argc, char **argv)
 			return crypt_wipe_key(cc);
 		}
 	}
-
-error:
+show_warning:
 	DMWARN("unrecognised message received.");
 	return -EINVAL;
 }