All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else
@ 2017-03-05 19:37 Tamara Diaconita
  2017-03-05 20:10 ` Samuel Thibault
  2017-03-06 11:29 ` Tamara Diaconita
  0 siblings, 2 replies; 7+ messages in thread
From: Tamara Diaconita @ 2017-03-05 19:37 UTC (permalink / raw)
  To: w.d.hubbs, chris, kirk, samuel.thibault, gregkh, outreachy-kernel
  Cc: Tamara Diaconita

Fixed the checkpath.pl WARNING:
Else is not generally useful after a break return.

Deleted the 'else' structure.
Deleted extra tabs which remained after the 'else' structure was
removed.

Signed-off-by: Tamara Diaconita <diaconita.tamara@gmail.com>
---
 drivers/staging/speakup/keyhelp.c | 44 +++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/speakup/keyhelp.c b/drivers/staging/speakup/keyhelp.c
index 2ef7370..0e0efc46 100644
--- a/drivers/staging/speakup/keyhelp.c
+++ b/drivers/staging/speakup/keyhelp.c
@@ -176,30 +176,30 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key)
 		synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO));
 		build_key_data(); /* rebuild each time in case new mapping */
 		return 1;
-	} else {
-		name = NULL;
-		if ((type != KT_SPKUP) && (key > 0) && (key <= num_key_names)) {
-			synth_printf("%s\n",
-				     spk_msg_get(MSG_KEYNAMES_START + key - 1));
-			return 1;
-		}
-		for (i = 0; funcvals[i] != 0 && !name; i++) {
-			if (ch == funcvals[i])
-				name = spk_msg_get(MSG_FUNCNAMES_START + i);
-		}
-		if (!name)
-			return -1;
-		kp = spk_our_keys[key] + 1;
-		for (i = 0; i < nstates; i++) {
-			if (ch == kp[i])
-				break;
-		}
-		key += (state_tbl[i] << 8);
-		say_key(key);
-		synth_printf(spk_msg_get(MSG_KEYDESC), name);
-		synth_printf("\n");
+	}
+	name = NULL;
+	if ((type != KT_SPKUP) && (key > 0) && (key <= num_key_names)) {
+		synth_printf("%s\n",
+			     spk_msg_get(MSG_KEYNAMES_START + key - 1));
 		return 1;
 	}
+	for (i = 0; funcvals[i] != 0 && !name; i++) {
+		if (ch == funcvals[i])
+			name = spk_msg_get(MSG_FUNCNAMES_START + i);
+	}
+	if (!name)
+		return -1;
+	kp = spk_our_keys[key] + 1;
+	for (i = 0; i < nstates; i++) {
+		if (ch == kp[i])
+			break;
+		}
+	key += (state_tbl[i] << 8);
+	say_key(key);
+	synth_printf(spk_msg_get(MSG_KEYDESC), name);
+	synth_printf("\n");
+	return 1;
+
 	name = spk_msg_get(MSG_FUNCNAMES_START + cur_item);
 	func = funcvals[cur_item];
 	synth_printf("%s", name);
-- 
2.9.3



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else
  2017-03-05 19:37 [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else Tamara Diaconita
@ 2017-03-05 20:10 ` Samuel Thibault
  2017-03-06 11:29 ` Tamara Diaconita
  1 sibling, 0 replies; 7+ messages in thread
From: Samuel Thibault @ 2017-03-05 20:10 UTC (permalink / raw)
  To: Tamara Diaconita
  Cc: w.d.hubbs, chris, kirk, gregkh, outreachy-kernel, Tamara Diaconita

Tamara Diaconita, on dim. 05 mars 2017 21:37:01 +0200, wrote:
> Fixed the checkpath.pl WARNING:
> Else is not generally useful after a break return.
> 
> Deleted the 'else' structure.
> Deleted extra tabs which remained after the 'else' structure was
> removed.

But not all if cases finish with a return, so it's not the same.

> ---
>  drivers/staging/speakup/keyhelp.c | 44 +++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/staging/speakup/keyhelp.c b/drivers/staging/speakup/keyhelp.c
> index 2ef7370..0e0efc46 100644
> --- a/drivers/staging/speakup/keyhelp.c
> +++ b/drivers/staging/speakup/keyhelp.c
> @@ -176,30 +176,30 @@ int spk_handle_help(struct vc_data *vc, u_char type, u_char ch, u_short key)
>  		synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO));
>  		build_key_data(); /* rebuild each time in case new mapping */
>  		return 1;
> -	} else {
> -		name = NULL;
> -		if ((type != KT_SPKUP) && (key > 0) && (key <= num_key_names)) {
> -			synth_printf("%s\n",
> -				     spk_msg_get(MSG_KEYNAMES_START + key - 1));
> -			return 1;
> -		}
> -		for (i = 0; funcvals[i] != 0 && !name; i++) {
> -			if (ch == funcvals[i])
> -				name = spk_msg_get(MSG_FUNCNAMES_START + i);
> -		}
> -		if (!name)
> -			return -1;
> -		kp = spk_our_keys[key] + 1;
> -		for (i = 0; i < nstates; i++) {
> -			if (ch == kp[i])
> -				break;
> -		}
> -		key += (state_tbl[i] << 8);
> -		say_key(key);
> -		synth_printf(spk_msg_get(MSG_KEYDESC), name);
> -		synth_printf("\n");
> +	}
> +	name = NULL;
> +	if ((type != KT_SPKUP) && (key > 0) && (key <= num_key_names)) {
> +		synth_printf("%s\n",
> +			     spk_msg_get(MSG_KEYNAMES_START + key - 1));
>  		return 1;
>  	}
> +	for (i = 0; funcvals[i] != 0 && !name; i++) {
> +		if (ch == funcvals[i])
> +			name = spk_msg_get(MSG_FUNCNAMES_START + i);
> +	}
> +	if (!name)
> +		return -1;
> +	kp = spk_our_keys[key] + 1;
> +	for (i = 0; i < nstates; i++) {
> +		if (ch == kp[i])
> +			break;
> +		}
> +	key += (state_tbl[i] << 8);
> +	say_key(key);
> +	synth_printf(spk_msg_get(MSG_KEYDESC), name);
> +	synth_printf("\n");
> +	return 1;
> +
>  	name = spk_msg_get(MSG_FUNCNAMES_START + cur_item);
>  	func = funcvals[cur_item];
>  	synth_printf("%s", name);
> -- 
> 2.9.3
> 

-- 
Samuel
<D> I hated the Mighty Mouse in the Apple Store every time I played with it. I hated the Mighty Mouse at work whenever I set up a Mac for somebody.
<D> I decided to give it one last chance when I set up my new Mac
<D> And golly, I like it at home.
<D> Maybe mine is defective in a way that makes it good.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else
  2017-03-05 19:37 [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else Tamara Diaconita
  2017-03-05 20:10 ` Samuel Thibault
@ 2017-03-06 11:29 ` Tamara Diaconita
  2017-03-06 11:41   ` [Outreachy kernel] " Julia Lawall
  2017-03-06 11:41   ` Daniel Baluta
  1 sibling, 2 replies; 7+ messages in thread
From: Tamara Diaconita @ 2017-03-06 11:29 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: w.d.hubbs, chris, kirk, samuel.thibault, gregkh, diaconita.tamara


[-- Attachment #1.1: Type: text/plain, Size: 3233 bytes --]

Did I do something wrong? In this case I see that the if statement finish 
with a return.

duminică, 5 martie 2017, 21:37:05 UTC+2, Tamara Diaconita a scris:
>
> Fixed the checkpath.pl WARNING: 
> Else is not generally useful after a break return. 
>
> Deleted the 'else' structure. 
> Deleted extra tabs which remained after the 'else' structure was 
> removed. 
>
> Signed-off-by: Tamara Diaconita <diaconita.tamara@gmail.com> 
> --- 
>  drivers/staging/speakup/keyhelp.c | 44 
> +++++++++++++++++++-------------------- 
>  1 file changed, 22 insertions(+), 22 deletions(-) 
>
> diff --git a/drivers/staging/speakup/keyhelp.c 
> b/drivers/staging/speakup/keyhelp.c 
> index 2ef7370..0e0efc46 100644 
> --- a/drivers/staging/speakup/keyhelp.c 
> +++ b/drivers/staging/speakup/keyhelp.c 
> @@ -176,30 +176,30 @@ int spk_handle_help(struct vc_data *vc, u_char type, 
> u_char ch, u_short key) 
>                  synth_printf("%s\n", spk_msg_get(MSG_HELP_INFO)); 
>                  build_key_data(); /* rebuild each time in case new 
> mapping */ 
>                  return 1; 
> -        } else { 
> -                name = NULL; 
> -                if ((type != KT_SPKUP) && (key > 0) && (key <= 
> num_key_names)) { 
> -                        synth_printf("%s\n", 
> -                                     spk_msg_get(MSG_KEYNAMES_START + key 
> - 1)); 
> -                        return 1; 
> -                } 
> -                for (i = 0; funcvals[i] != 0 && !name; i++) { 
> -                        if (ch == funcvals[i]) 
> -                                name = spk_msg_get(MSG_FUNCNAMES_START + 
> i); 
> -                } 
> -                if (!name) 
> -                        return -1; 
> -                kp = spk_our_keys[key] + 1; 
> -                for (i = 0; i < nstates; i++) { 
> -                        if (ch == kp[i]) 
> -                                break; 
> -                } 
> -                key += (state_tbl[i] << 8); 
> -                say_key(key); 
> -                synth_printf(spk_msg_get(MSG_KEYDESC), name); 
> -                synth_printf("\n"); 
> +        } 
> +        name = NULL; 
> +        if ((type != KT_SPKUP) && (key > 0) && (key <= num_key_names)) { 
> +                synth_printf("%s\n", 
> +                             spk_msg_get(MSG_KEYNAMES_START + key - 1)); 
>                  return 1; 
>          } 
> +        for (i = 0; funcvals[i] != 0 && !name; i++) { 
> +                if (ch == funcvals[i]) 
> +                        name = spk_msg_get(MSG_FUNCNAMES_START + i); 
> +        } 
> +        if (!name) 
> +                return -1; 
> +        kp = spk_our_keys[key] + 1; 
> +        for (i = 0; i < nstates; i++) { 
> +                if (ch == kp[i]) 
> +                        break; 
> +                } 
> +        key += (state_tbl[i] << 8); 
> +        say_key(key); 
> +        synth_printf(spk_msg_get(MSG_KEYDESC), name); 
> +        synth_printf("\n"); 
> +        return 1; 
> + 
>          name = spk_msg_get(MSG_FUNCNAMES_START + cur_item); 
>          func = funcvals[cur_item]; 
>          synth_printf("%s", name); 
> -- 
> 2.9.3 
>
>

[-- Attachment #1.2: Type: text/html, Size: 5144 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else
  2017-03-06 11:29 ` Tamara Diaconita
@ 2017-03-06 11:41   ` Julia Lawall
  2017-03-06 13:46     ` Tamara Diaconita
  2017-03-06 11:41   ` Daniel Baluta
  1 sibling, 1 reply; 7+ messages in thread
From: Julia Lawall @ 2017-03-06 11:41 UTC (permalink / raw)
  To: Tamara Diaconita
  Cc: outreachy-kernel, w.d.hubbs, chris, kirk, samuel.thibault,
	gregkh, diaconita.tamara

[-- Attachment #1: Type: text/plain, Size: 5451 bytes --]



On Mon, 6 Mar 2017, Tamara Diaconita wrote:

> Did I do something wrong? In this case I see that the if statement finish
> with a return.

The structure is complex is this case.  For a simpler example, suppose you
have

if (x)
  print "hello"
else if (y)
  return;
else print "goodbye"

The original program prints hello or goodbye (or nothing), but not both.
If you remove the final else, it may print hello goodbye.  The structure
of your code is like this.  The KT_LATIN and KT_CUR cases might not
return.

Chckpatch just looks at very local structure.  It doesn't follow the
entire control flow, so it can have false positives like this.

julia

>
> duminică, 5 martie 2017, 21:37:05 UTC+2, Tamara Diaconita a scris:
>       Fixed the checkpath.pl WARNING:
>       Else is not generally useful after a break return.
>
>       Deleted the 'else' structure.
>       Deleted extra tabs which remained after the 'else' structure was
>       removed.
>
>       Signed-off-by: Tamara Diaconita <diaconita.tamara@gmail.com>
>       ---
>        drivers/staging/speakup/keyhelp.c | 44
>       +++++++++++++++++++--------------------
>        1 file changed, 22 insertions(+), 22 deletions(-)
>
>       diff --git a/drivers/staging/speakup/keyhelp.c
>       b/drivers/staging/speakup/keyhelp.c
>       index 2ef7370..0e0efc46 100644
>       --- a/drivers/staging/speakup/keyhelp.c
>       +++ b/drivers/staging/speakup/keyhelp.c
>       @@ -176,30 +176,30 @@ int spk_handle_help(struct vc_data *vc,
>       u_char type, u_char ch, u_short key)
>                        synth_printf("%s\n",
>       spk_msg_get(MSG_HELP_INFO));
>                        build_key_data(); /* rebuild each time in case
>       new mapping */
>                        return 1;
>       -        } else {
>       -                name = NULL;
>       -                if ((type != KT_SPKUP) && (key > 0) && (key <=
>       num_key_names)) {
>       -                        synth_printf("%s\n",
>       -                                    
>       spk_msg_get(MSG_KEYNAMES_START + key - 1));
>       -                        return 1;
>       -                }
>       -                for (i = 0; funcvals[i] != 0 && !name; i++) {
>       -                        if (ch == funcvals[i])
>       -                                name =
>       spk_msg_get(MSG_FUNCNAMES_START + i);
>       -                }
>       -                if (!name)
>       -                        return -1;
>       -                kp = spk_our_keys[key] + 1;
>       -                for (i = 0; i < nstates; i++) {
>       -                        if (ch == kp[i])
>       -                                break;
>       -                }
>       -                key += (state_tbl[i] << 8);
>       -                say_key(key);
>       -                synth_printf(spk_msg_get(MSG_KEYDESC), name);
>       -                synth_printf("\n");
>       +        }
>       +        name = NULL;
>       +        if ((type != KT_SPKUP) && (key > 0) && (key <=
>       num_key_names)) {
>       +                synth_printf("%s\n",
>       +                             spk_msg_get(MSG_KEYNAMES_START +
>       key - 1));
>                        return 1;
>                }
>       +        for (i = 0; funcvals[i] != 0 && !name; i++) {
>       +                if (ch == funcvals[i])
>       +                        name = spk_msg_get(MSG_FUNCNAMES_START
>       + i);
>       +        }
>       +        if (!name)
>       +                return -1;
>       +        kp = spk_our_keys[key] + 1;
>       +        for (i = 0; i < nstates; i++) {
>       +                if (ch == kp[i])
>       +                        break;
>       +                }
>       +        key += (state_tbl[i] << 8);
>       +        say_key(key);
>       +        synth_printf(spk_msg_get(MSG_KEYDESC), name);
>       +        synth_printf("\n");
>       +        return 1;
>       +
>                name = spk_msg_get(MSG_FUNCNAMES_START + cur_item);
>                func = funcvals[cur_item];
>                synth_printf("%s", name);
>       --
>       2.9.3
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/ae948b29-c3d0-40d3-94a0-
> 31df90500e76%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else
  2017-03-06 11:29 ` Tamara Diaconita
  2017-03-06 11:41   ` [Outreachy kernel] " Julia Lawall
@ 2017-03-06 11:41   ` Daniel Baluta
  1 sibling, 0 replies; 7+ messages in thread
From: Daniel Baluta @ 2017-03-06 11:41 UTC (permalink / raw)
  To: Tamara Diaconita
  Cc: outreachy-kernel, w.d.hubbs, chris, kirk, samuel.thibault,
	Greg Kroah-Hartman, Tamara Diaconita

On Mon, Mar 6, 2017 at 1:29 PM, Tamara Diaconita
<diaconitatamara@gmail.com> wrote:
> Did I do something wrong? In this case I see that the if statement finish
> with a return.

Hi Tamara,

Firstly, use bottom posting. Meaning that your reply should be under the text
you are replying to. Like this one.

Secondly, after cosmetic patches the logic of the code shouldn't be changed.
In your case it is changed.

The code looks like this:

if (cond1) {
/* code1 */
} else if (cond2) {
/* code2 */
} else if(cond3) {
/*code3 */
} else {
/* code4 */
}

/* code 5 */
Your code removes the last "else" which would be fine if all other
code path would end with a return.

But if you look into the code there is a path that doesn't end with a return [1]
and thus you break the initial logic.

The path that could end up without a return is this:

»       } else if (type == KT_CUR) {
»       »       if (ch == 0
»       »           && (MSG_FUNCNAMES_START + cur_item + 1) <=
»       »           MSG_FUNCNAMES_END)
»       »       »       cur_item++;
»       »       else if (ch == 3 && cur_item > 0)
»       »       »       cur_item--;
»       »       else
»       »       »       return -1;


Hope this helps.

thanks,
Daniel.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else
  2017-03-06 11:41   ` [Outreachy kernel] " Julia Lawall
@ 2017-03-06 13:46     ` Tamara Diaconita
  2017-03-06 13:50       ` Julia Lawall
  0 siblings, 1 reply; 7+ messages in thread
From: Tamara Diaconita @ 2017-03-06 13:46 UTC (permalink / raw)
  To: outreachy-kernel
  Cc: diaconitatamara, w.d.hubbs, chris, kirk, samuel.thibault, gregkh,
	diaconita.tamara


[-- Attachment #1.1: Type: text/plain, Size: 5321 bytes --]

Thank you for your explanations! 
Should I resend the patch set without this patch?

luni, 6 martie 2017, 13:41:29 UTC+2, Julia Lawall a scris:
>
>
>
> On Mon, 6 Mar 2017, Tamara Diaconita wrote: 
>
> > Did I do something wrong? In this case I see that the if statement 
> finish 
> > with a return. 
>
> The structure is complex is this case.  For a simpler example, suppose you 
> have 
>
> if (x) 
>   print "hello" 
> else if (y) 
>   return; 
> else print "goodbye" 
>
> The original program prints hello or goodbye (or nothing), but not both. 
> If you remove the final else, it may print hello goodbye.  The structure 
> of your code is like this.  The KT_LATIN and KT_CUR cases might not 
> return. 
>
> Chckpatch just looks at very local structure.  It doesn't follow the 
> entire control flow, so it can have false positives like this. 
>
> julia 
>
> > 
> > duminică, 5 martie 2017, 21:37:05 UTC+2, Tamara Diaconita a scris: 
> >       Fixed the checkpath.pl WARNING: 
> >       Else is not generally useful after a break return. 
> > 
> >       Deleted the 'else' structure. 
> >       Deleted extra tabs which remained after the 'else' structure was 
> >       removed. 
> > 
> >       Signed-off-by: Tamara Diaconita <diaconit...@gmail.com 
> <javascript:>> 
> >       --- 
> >        drivers/staging/speakup/keyhelp.c | 44 
> >       +++++++++++++++++++-------------------- 
> >        1 file changed, 22 insertions(+), 22 deletions(-) 
> > 
> >       diff --git a/drivers/staging/speakup/keyhelp.c 
> >       b/drivers/staging/speakup/keyhelp.c 
> >       index 2ef7370..0e0efc46 100644 
> >       --- a/drivers/staging/speakup/keyhelp.c 
> >       +++ b/drivers/staging/speakup/keyhelp.c 
> >       @@ -176,30 +176,30 @@ int spk_handle_help(struct vc_data *vc, 
> >       u_char type, u_char ch, u_short key) 
> >                        synth_printf("%s\n", 
> >       spk_msg_get(MSG_HELP_INFO)); 
> >                        build_key_data(); /* rebuild each time in case 
> >       new mapping */ 
> >                        return 1; 
> >       -        } else { 
> >       -                name = NULL; 
> >       -                if ((type != KT_SPKUP) && (key > 0) && (key <= 
> >       num_key_names)) { 
> >       -                        synth_printf("%s\n", 
> >       -                                     
> >       spk_msg_get(MSG_KEYNAMES_START + key - 1)); 
> >       -                        return 1; 
> >       -                } 
> >       -                for (i = 0; funcvals[i] != 0 && !name; i++) { 
> >       -                        if (ch == funcvals[i]) 
> >       -                                name = 
> >       spk_msg_get(MSG_FUNCNAMES_START + i); 
> >       -                } 
> >       -                if (!name) 
> >       -                        return -1; 
> >       -                kp = spk_our_keys[key] + 1; 
> >       -                for (i = 0; i < nstates; i++) { 
> >       -                        if (ch == kp[i]) 
> >       -                                break; 
> >       -                } 
> >       -                key += (state_tbl[i] << 8); 
> >       -                say_key(key); 
> >       -                synth_printf(spk_msg_get(MSG_KEYDESC), name); 
> >       -                synth_printf("\n"); 
> >       +        } 
> >       +        name = NULL; 
> >       +        if ((type != KT_SPKUP) && (key > 0) && (key <= 
> >       num_key_names)) { 
> >       +                synth_printf("%s\n", 
> >       +                             spk_msg_get(MSG_KEYNAMES_START + 
> >       key - 1)); 
> >                        return 1; 
> >                } 
> >       +        for (i = 0; funcvals[i] != 0 && !name; i++) { 
> >       +                if (ch == funcvals[i]) 
> >       +                        name = spk_msg_get(MSG_FUNCNAMES_START 
> >       + i); 
> >       +        } 
> >       +        if (!name) 
> >       +                return -1; 
> >       +        kp = spk_our_keys[key] + 1; 
> >       +        for (i = 0; i < nstates; i++) { 
> >       +                if (ch == kp[i]) 
> >       +                        break; 
> >       +                } 
> >       +        key += (state_tbl[i] << 8); 
> >       +        say_key(key); 
> >       +        synth_printf(spk_msg_get(MSG_KEYDESC), name); 
> >       +        synth_printf("\n"); 
> >       +        return 1; 
> >       + 
> >                name = spk_msg_get(MSG_FUNCNAMES_START + cur_item); 
> >                func = funcvals[cur_item]; 
> >                synth_printf("%s", name); 
> >       -- 
> >       2.9.3 
> > 
> > -- 
> > You received this message because you are subscribed to the Google 
> Groups 
> > "outreachy-kernel" group. 
> > To unsubscribe from this group and stop receiving emails from it, send 
> an 
> > email to outreachy-kern...@googlegroups.com <javascript:>. 
> > To post to this group, send email to outreach...@googlegroups.com 
> <javascript:>. 
> > To view this discussion on the web visithttps://
> groups.google.com/d/msgid/outreachy-kernel/ae948b29-c3d0-40d3-94a0- 
> > 31df90500e76%40googlegroups.com. 
> > For more options, visit https://groups.google.com/d/optout. 
> > 
> >


[-- Attachment #1.2: Type: text/html, Size: 9264 bytes --]

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else
  2017-03-06 13:46     ` Tamara Diaconita
@ 2017-03-06 13:50       ` Julia Lawall
  0 siblings, 0 replies; 7+ messages in thread
From: Julia Lawall @ 2017-03-06 13:50 UTC (permalink / raw)
  To: Tamara Diaconita
  Cc: outreachy-kernel, w.d.hubbs, chris, kirk, samuel.thibault,
	gregkh, diaconita.tamara

[-- Attachment #1: Type: text/plain, Size: 7493 bytes --]



On Mon, 6 Mar 2017, Tamara Diaconita wrote:

> Thank you for your explanations!
> Should I resend the patch set without this patch?

OK

julia

>
> luni, 6 martie 2017, 13:41:29 UTC+2, Julia Lawall a scris:
>
>
>       On Mon, 6 Mar 2017, Tamara Diaconita wrote:
>
>       > Did I do something wrong? In this case I see that the if
>       statement finish
>       > with a return.
>
>       The structure is complex is this case.  For a simpler example,
>       suppose you
>       have
>
>       if (x)
>         print "hello"
>       else if (y)
>         return;
>       else print "goodbye"
>
>       The original program prints hello or goodbye (or nothing), but
>       not both.
>       If you remove the final else, it may print hello goodbye.  The
>       structure
>       of your code is like this.  The KT_LATIN and KT_CUR cases might
>       not
>       return.
>
>       Chckpatch just looks at very local structure.  It doesn't follow
>       the
>       entire control flow, so it can have false positives like this.
>
>       julia
>
>       >
>       > duminică, 5 martie 2017, 21:37:05 UTC+2, Tamara Diaconita a
>       scris:
>       >       Fixed the checkpath.pl WARNING:
>       >       Else is not generally useful after a break return.
>       >
>       >       Deleted the 'else' structure.
>       >       Deleted extra tabs which remained after the 'else'
>       structure was
>       >       removed.
>       >
>       >       Signed-off-by: Tamara Diaconita <diaconit...@gmail.com>
>       >       ---
>       >        drivers/staging/speakup/keyhelp.c | 44
>       >       +++++++++++++++++++--------------------
>       >        1 file changed, 22 insertions(+), 22 deletions(-)
>       >
>       >       diff --git a/drivers/staging/speakup/keyhelp.c
>       >       b/drivers/staging/speakup/keyhelp.c
>       >       index 2ef7370..0e0efc46 100644
>       >       --- a/drivers/staging/speakup/keyhelp.c
>       >       +++ b/drivers/staging/speakup/keyhelp.c
>       >       @@ -176,30 +176,30 @@ int spk_handle_help(struct vc_data
>       *vc,
>       >       u_char type, u_char ch, u_short key)
>       >                        synth_printf("%s\n",
>       >       spk_msg_get(MSG_HELP_INFO));
>       >                        build_key_data(); /* rebuild each time
>       in case
>       >       new mapping */
>       >                        return 1;
>       >       -        } else {
>       >       -                name = NULL;
>       >       -                if ((type != KT_SPKUP) && (key > 0) &&
>       (key <=
>       >       num_key_names)) {
>       >       -                        synth_printf("%s\n",
>       >       -                                    
>       >       spk_msg_get(MSG_KEYNAMES_START + key - 1));
>       >       -                        return 1;
>       >       -                }
>       >       -                for (i = 0; funcvals[i] != 0 && !name;
>       i++) {
>       >       -                        if (ch == funcvals[i])
>       >       -                                name =
>       >       spk_msg_get(MSG_FUNCNAMES_START + i);
>       >       -                }
>       >       -                if (!name)
>       >       -                        return -1;
>       >       -                kp = spk_our_keys[key] + 1;
>       >       -                for (i = 0; i < nstates; i++) {
>       >       -                        if (ch == kp[i])
>       >       -                                break;
>       >       -                }
>       >       -                key += (state_tbl[i] << 8);
>       >       -                say_key(key);
>       >       -                synth_printf(spk_msg_get(MSG_KEYDESC),
>       name);
>       >       -                synth_printf("\n");
>       >       +        }
>       >       +        name = NULL;
>       >       +        if ((type != KT_SPKUP) && (key > 0) && (key <=
>       >       num_key_names)) {
>       >       +                synth_printf("%s\n",
>       >       +                            
>       spk_msg_get(MSG_KEYNAMES_START +
>       >       key - 1));
>       >                        return 1;
>       >                }
>       >       +        for (i = 0; funcvals[i] != 0 && !name; i++) {
>       >       +                if (ch == funcvals[i])
>       >       +                        name =
>       spk_msg_get(MSG_FUNCNAMES_START
>       >       + i);
>       >       +        }
>       >       +        if (!name)
>       >       +                return -1;
>       >       +        kp = spk_our_keys[key] + 1;
>       >       +        for (i = 0; i < nstates; i++) {
>       >       +                if (ch == kp[i])
>       >       +                        break;
>       >       +                }
>       >       +        key += (state_tbl[i] << 8);
>       >       +        say_key(key);
>       >       +        synth_printf(spk_msg_get(MSG_KEYDESC), name);
>       >       +        synth_printf("\n");
>       >       +        return 1;
>       >       +
>       >                name = spk_msg_get(MSG_FUNCNAMES_START +
>       cur_item);
>       >                func = funcvals[cur_item];
>       >                synth_printf("%s", name);
>       >       --
>       >       2.9.3
>       >
>       > --
>       > You received this message because you are subscribed to the
>       Google Groups
>       > "outreachy-kernel" group.
>       > To unsubscribe from this group and stop receiving emails from
>       it, send an
>       > email to outreachy-kern...@googlegroups.com.
>       > To post to this group, send email to
>       outreach...@googlegroups.com.
>       > To view this discussion on the webvisithttps://groups.google.com/d/msgid/outreachy-kernel/ae948b29-c3d0-40d3-
>       94a0-
>       > 31df90500e76%40googlegroups.com.
>       > For more options, visit https://groups.google.com/d/optout.
>       >
>       >
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/4efe0441-12da-4574-bd58-
> 0a5a95af2977%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-03-06 13:50 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-05 19:37 [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else Tamara Diaconita
2017-03-05 20:10 ` Samuel Thibault
2017-03-06 11:29 ` Tamara Diaconita
2017-03-06 11:41   ` [Outreachy kernel] " Julia Lawall
2017-03-06 13:46     ` Tamara Diaconita
2017-03-06 13:50       ` Julia Lawall
2017-03-06 11:41   ` Daniel Baluta

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.