Re: [patch] CodingStyle: add some more error handling guidelines

Re: [patch] CodingStyle: add some more error handling guidelines

[SF Markus Elfring]

> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
> index 9f28b14..9c8a234 100644
> --- a/Documentation/CodingStyle
> +++ b/Documentation/CodingStyle
> @@ -392,7 +392,12 @@ The goto statement comes in handy when a function exits from multiple
>  locations and some common work such as cleanup has to be done.  If there is no
>  cleanup needed then just return directly.
>  
> -The rationale is:
> +Choose label names which say what the goto does or why the goto exists.  An
> +[...]  Avoid
> +using GW-BASIC names like "err1:" and "err2:".  Also don't name them after the
> +goto location like "err_kmalloc_failed:"

I find this documentation approach not safe and clear enough so far.

* How should the reference to an other programming language help in the understanding
  of the recommended naming convention for jump labels?

* To which source code place should the word "location" refer to?
  - jump source
  - jump target

Regards,
Markus

Re: [patch] CodingStyle: add some more error handling guidelines

[Arend van Spriel]

On 12/03/14 13:31, SF Markus Elfring wrote:
>> diff --git a/Documentation/CodingStyle b/Documentation/CodingStyle
>> index 9f28b14..9c8a234 100644
>> --- a/Documentation/CodingStyle
>> +++ b/Documentation/CodingStyle
>> @@ -392,7 +392,12 @@ The goto statement comes in handy when a function exits from multiple
>>   locations and some common work such as cleanup has to be done.  If there is no
>>   cleanup needed then just return directly.
>>
>> -The rationale is:
>> +Choose label names which say what the goto does or why the goto exists.  An
>> +[...]  Avoid
>> +using GW-BASIC names like "err1:" and "err2:".  Also don't name them after the
>> +goto location like "err_kmalloc_failed:"
>
> I find this documentation approach not safe and clear enough so far.
>
> * How should the reference to an other programming language help in the understanding
>    of the recommended naming convention for jump labels?
>
> * To which source code place should the word "location" refer to?
>    - jump source
>    - jump target

I think you digested the paragraph in too small bits. The term "goto 
location" looks synonymous to "jump source" to me.

Regards,
Arend

> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe backports" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] CodingStyle: add some more error handling guidelines

[SF Markus Elfring]

>> * To which source code place should the word "location" refer to?
>>    - jump source
>>    - jump target
> 
> I think you digested the paragraph in too small bits.

I would prefer to reduce the probability for misunderstandings of the 
proposed wording a bit more.


> The term "goto location" looks synonymous to "jump source" to me.

I would interpret it differently because of the specific placement of this key word
before an other term.

Regards,
Markus

Re: [patch] CodingStyle: add some more error handling guidelines

[Dan Carpenter]

On Wed, Dec 03, 2014 at 01:31:19PM +0100, SF Markus Elfring wrote:
> 
> * To which source code place should the word "location" refer to?
>   - jump source
>   - jump target

jump target.

regards,
dan carpenter

Re: [patch] CodingStyle: add some more error handling guidelines

[Julia Lawall]

On Wed, 3 Dec 2014, Dan Carpenter wrote:

> On Wed, Dec 03, 2014 at 01:31:19PM +0100, SF Markus Elfring wrote:
> >
> > * To which source code place should the word "location" refer to?
> >   - jump source
> >   - jump target
>
> jump target.

I think you mean source?  Or it really is ambiguous.  The example was
err_kmalloc_failed, which seems source-related.

julia

Re: [patch] CodingStyle: add some more error handling guidelines

[Dan Carpenter]

On Wed, Dec 03, 2014 at 01:52:53PM +0100, Julia Lawall wrote:
> On Wed, 3 Dec 2014, Dan Carpenter wrote:
> 
> > On Wed, Dec 03, 2014 at 01:31:19PM +0100, SF Markus Elfring wrote:
> > >
> > > * To which source code place should the word "location" refer to?
> > >   - jump source
> > >   - jump target
> >
> > jump target.
> 
> I think you mean source?  Or it really is ambiguous.  The example was
> err_kmalloc_failed, which seems source-related.

Yeah.  You're right.  I misread Markus's email.  The goto location is
where the goto is.  The label location is where the label is.

regards,
dan carpenter

Re: [patch] CodingStyle: add some more error handling guidelines

[SF Markus Elfring]

>> * To which source code place should the word "location" refer to?
>>   - jump source
>>   - jump target
> 
> jump target.

Thanks for the clarification of your intention.

I wonder then why I got the feedback "That is a useless thing to do."
from you yesterday.
I hope that we can still clarify our different opinions about specific
implementation details in constructive ways.

Regards,
Markus

Re: [patch] CodingStyle: add some more error handling guidelines

[Dan Carpenter]

Sorry.  I misread your email.  If the code looks like this:

	foo = kmalloc();
	if (!foo)
		goto kmalloc_failed;

The "kmalloc_failed" doesn't add any information.  We can see that
kmalloc failed from the context.

regards,
dan carpenter

Re: [patch] CodingStyle: add some more error handling guidelines

[SF Markus Elfring]

> Sorry.  I misread your email.  If the code looks like this:
> 
> 	foo = kmalloc();
> 	if (!foo)
> 		goto kmalloc_failed;
> 
> The "kmalloc_failed" doesn't add any information.

I find that this such a name approach would fit to your
expectation of a source-oriented labeling of these identifiers.


> We can see that kmalloc failed from the context.

Which name pattern do you find more appropriate in such
an use case?

Regards,
Markus

Re: [patch] CodingStyle: add some more error handling guidelines

[Arend van Spriel]

On 12/03/14 14:24, SF Markus Elfring wrote:
>> Sorry.  I misread your email.  If the code looks like this:
>>
>> 	foo = kmalloc();
>> 	if (!foo)
>> 		goto kmalloc_failed;
>>
>> The "kmalloc_failed" doesn't add any information.
>
> I find that this such a name approach would fit to your
> expectation of a source-oriented labeling of these identifiers.
>
>
>> We can see that kmalloc failed from the context.
>
> Which name pattern do you find more appropriate in such
> an use case?

I think Dan wants the label to be descriptive about the tasks needed in 
the exception handling itself. This makes sense as the exception 
handling steps may be reused for different failures in the code.

void foo(void)
{
	if (check_a())
		goto do_bar;

	sub_foo1();

	if (checck_b())
		goto do_bar;

	sub_foo2();
	return;

do_bar:
	bar();
}

Regards,
Arend

> Regards,
> Markus
> --
> To unsubscribe from this list: send the line "unsubscribe backports" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch] CodingStyle: add some more error handling guidelines

[SF Markus Elfring]

>> Which name pattern do you find more appropriate in such
>> an use case?
> 
> I think Dan wants the label to be descriptive about the tasks
> needed in the exception handling itself.

I would usually prefer also such a target-oriented labelling
for the affected identifiers.
How are the chances to express an expectation in this direction
unambiguously for the proposed coding style update?


> This makes sense as the exception handling steps may be reused
> for different failures in the code.

I would stress a different reason from my point of view.

Regards,
Markus

Re: [patch] CodingStyle: add some more error handling guidelines

[Arend van Spriel]

On 12/03/14 17:00, SF Markus Elfring wrote:
>>> Which name pattern do you find more appropriate in such
>>> an use case?
>>
>> I think Dan wants the label to be descriptive about the tasks
>> needed in the exception handling itself.
>
> I would usually prefer also such a target-oriented labelling
> for the affected identifiers.
> How are the chances to express an expectation in this direction
> unambiguously for the proposed coding style update?
>
>
>> This makes sense as the exception handling steps may be reused
>> for different failures in the code.
>
> I would stress a different reason from my point of view.

I meant as apposed to using a goto-/source-oriented labelling. Please 
provide your point of view. That way the explanations given in this 
email exchange might be incorporated in the next round of the proposed 
update or at least be used as input.

Regards,
Arend

Re: [patch] CodingStyle: add some more error handling guidelines

[SF Markus Elfring]

> Please provide your point of view.

I would like to interpret the key word "goto" from the C programming
language a bit more here so that a better common understanding can
eventually be achieved.

Strong opinions might be floating around for the consistent naming
of jump labels. My reasoning works like the following.

This key word could also be interpreted as two items "go" and "to",
couldn't it?
How much does this variation stress its meaning in a specific direction?

Some software developers would like to express the reason about
an unexpected event at the jump source. But I guess that this approach
increases the risk for a popular story like "goto fail;", doesn't it?
I would prefer not to specify "go to failure".

So I find that there are more variants possible to stress the jump target.
Examples:
* Failure_exit
* out_memory_release
* unregister_item

Regards,
Markus