| Message ID | 285954ec-280f-8a5a-5189-eb2471b4339c@users.sourceforge.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
October 14, 2016 1:43 PM, "SF Markus Elfring" <elfring@users.sourceforge.net> wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Fri, 14 Oct 2016 10:40:12 +0200 > > Move the setting for the local variables "mask", "match" and "rc6_csl" > behind the source code for a condition check by this function > at the beginning. Again, I can't see what the point is? > Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> > --- > drivers/media/rc/winbond-cir.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c > index fd997f0..9d05e17 100644 > --- a/drivers/media/rc/winbond-cir.c > +++ b/drivers/media/rc/winbond-cir.c > @@ -702,17 +702,17 @@ wbcir_shutdown(struct pnp_dev *device) > bool do_wake = true; > u8 match[11]; > u8 mask[11]; > - u8 rc6_csl = 0; > + u8 rc6_csl; > int i; > > - memset(match, 0, sizeof(match)); > - memset(mask, 0, sizeof(mask)); > - > if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) { > do_wake = false; > goto finish; > } > > + rc6_csl = 0; > + memset(match, 0, sizeof(match)); > + memset(mask, 0, sizeof(mask)); > switch (protocol) { > case IR_PROTOCOL_RC5: > if (wake_sc > 0xFFF) { > -- > 2.10.1 -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Move the setting for the local variables "mask", "match" and "rc6_csl" >> behind the source code for a condition check by this function >> at the beginning. > > Again, I can't see what the point is? * How do you think about to set these variables only after the initial check succeded? * Do you care for data access locality? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
October 19, 2016 3:38 PM, "SF Markus Elfring" <elfring@users.sourceforge.net> wrote: >>> Move the setting for the local variables "mask", "match" and "rc6_csl" >>> behind the source code for a condition check by this function >>> at the beginning. >> >> Again, I can't see what the point is? > > * How do you think about to set these variables only after the initial > check succeded? I prefer setting variables early so that no thinking about whether they're initialized or not is necessary later. > * Do you care for data access locality? Not unless you can show measurable performance improvements? -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>> Move the setting for the local variables "mask", "match" and "rc6_csl" >>>> behind the source code for a condition check by this function >>>> at the beginning. >>> >>> Again, I can't see what the point is? >> >> * How do you think about to set these variables only after the initial >> check succeded? > > I prefer setting variables early so that no thinking about > whether they're initialized or not is necessary later. * How do you think about to reduce the scope for these variables then? * Would you dare to move the corresponding source code into a separate function? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/rc/winbond-cir.c b/drivers/media/rc/winbond-cir.c index fd997f0..9d05e17 100644 --- a/drivers/media/rc/winbond-cir.c +++ b/drivers/media/rc/winbond-cir.c @@ -702,17 +702,17 @@ wbcir_shutdown(struct pnp_dev *device) bool do_wake = true; u8 match[11]; u8 mask[11]; - u8 rc6_csl = 0; + u8 rc6_csl; int i; - memset(match, 0, sizeof(match)); - memset(mask, 0, sizeof(mask)); - if (wake_sc == INVALID_SCANCODE || !device_may_wakeup(dev)) { do_wake = false; goto finish; } + rc6_csl = 0; + memset(match, 0, sizeof(match)); + memset(mask, 0, sizeof(mask)); switch (protocol) { case IR_PROTOCOL_RC5: if (wake_sc > 0xFFF) {