Re: [Outreachy kernel] Re: [PATCH 3/3] staging:speakup:keyhelp.c: Remove unnecessary else

[Julia Lawall <julia.lawall@lip6.fr>]

[-- 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.
>
>