| Message ID | ebd316f3f4f7cefa937562adba8ce60f2057ca9d.1417015567.git.mchehab@osg.samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 26, 2014 at 10:26 AM, Mauro Carvalho Chehab <mchehab@osg.samsung.com> wrote: > As reported by smatch: > drivers/media/tuners/tda18271-common.c:176 tda18271_read_extended() warn: if statement not indented > > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > > diff --git a/drivers/media/tuners/tda18271-common.c b/drivers/media/tuners/tda18271-common.c > index 86e5e3110118..6118203543ea 100644 > --- a/drivers/media/tuners/tda18271-common.c > +++ b/drivers/media/tuners/tda18271-common.c > @@ -173,12 +173,9 @@ int tda18271_read_extended(struct dvb_frontend *fe) > > for (i = 0; i < TDA18271_NUM_REGS; i++) { > /* don't update write-only registers */ > - if ((i != R_EB9) && > - (i != R_EB16) && > - (i != R_EB17) && > - (i != R_EB19) && > - (i != R_EB20)) > - regs[i] = regdump[i]; > + if ((i != R_EB9) && (i != R_EB16) && (i != R_EB17) && > + (i != R_EB19) && (i != R_EB20)) > + regs[i] = regdump[i]; > } > > if (tda18271_debug & DBG_REG) > -- > 1.9.3 > Mauro, I would actually rather NOT merge this patch. This hurts the readability of the code. If applied already, please revert it. Cheers, Mike -- 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
Em Thu, 27 Nov 2014 13:47:09 -0500 Michael Ira Krufky <mkrufky@linuxtv.org> escreveu: > On Wed, Nov 26, 2014 at 10:26 AM, Mauro Carvalho Chehab > <mchehab@osg.samsung.com> wrote: > > As reported by smatch: > > drivers/media/tuners/tda18271-common.c:176 tda18271_read_extended() warn: if statement not indented > > > > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> > > > > diff --git a/drivers/media/tuners/tda18271-common.c b/drivers/media/tuners/tda18271-common.c > > index 86e5e3110118..6118203543ea 100644 > > --- a/drivers/media/tuners/tda18271-common.c > > +++ b/drivers/media/tuners/tda18271-common.c > > @@ -173,12 +173,9 @@ int tda18271_read_extended(struct dvb_frontend *fe) > > > > for (i = 0; i < TDA18271_NUM_REGS; i++) { > > /* don't update write-only registers */ > > - if ((i != R_EB9) && > > - (i != R_EB16) && > > - (i != R_EB17) && > > - (i != R_EB19) && > > - (i != R_EB20)) > > - regs[i] = regdump[i]; > > + if ((i != R_EB9) && (i != R_EB16) && (i != R_EB17) && > > + (i != R_EB19) && (i != R_EB20)) > > + regs[i] = regdump[i]; > > } > > > > if (tda18271_debug & DBG_REG) > > -- > > 1.9.3 > > > > Mauro, > > I would actually rather NOT merge this patch. This hurts the > readability of the code. If applied already, please revert it. What hurts readability is to not indent regs[i] = regdump[i]; > > Cheers, > > Mike -- 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
On Thu, Nov 27, 2014 at 1:59 PM, Mauro Carvalho Chehab <mchehab@osg.samsung.com> wrote: > Em Thu, 27 Nov 2014 13:47:09 -0500 > Michael Ira Krufky <mkrufky@linuxtv.org> escreveu: > >> On Wed, Nov 26, 2014 at 10:26 AM, Mauro Carvalho Chehab >> <mchehab@osg.samsung.com> wrote: >> > As reported by smatch: >> > drivers/media/tuners/tda18271-common.c:176 tda18271_read_extended() warn: if statement not indented >> > >> > Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> >> > >> > diff --git a/drivers/media/tuners/tda18271-common.c b/drivers/media/tuners/tda18271-common.c >> > index 86e5e3110118..6118203543ea 100644 >> > --- a/drivers/media/tuners/tda18271-common.c >> > +++ b/drivers/media/tuners/tda18271-common.c >> > @@ -173,12 +173,9 @@ int tda18271_read_extended(struct dvb_frontend *fe) >> > >> > for (i = 0; i < TDA18271_NUM_REGS; i++) { >> > /* don't update write-only registers */ >> > - if ((i != R_EB9) && >> > - (i != R_EB16) && >> > - (i != R_EB17) && >> > - (i != R_EB19) && >> > - (i != R_EB20)) >> > - regs[i] = regdump[i]; >> > + if ((i != R_EB9) && (i != R_EB16) && (i != R_EB17) && >> > + (i != R_EB19) && (i != R_EB20)) >> > + regs[i] = regdump[i]; >> > } >> > >> > if (tda18271_debug & DBG_REG) >> > -- >> > 1.9.3 >> > >> >> Mauro, >> >> I would actually rather NOT merge this patch. This hurts the >> readability of the code. If applied already, please revert it. > > What hurts readability is to not indent regs[i] = regdump[i]; > >> >> Cheers, >> >> Mike If the patch were only fixing the indent of "regs[i] = regdump[i];" then it wouldn't bother me. I don't approve of the whitespace change in the if statement. Please resubmit it as a one-liner that *only* fixes the single bad indentation of the assignment to regs[i]. Cheers, Mike -- 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
On 11/27/2014 09:04 PM, Michael Ira Krufky wrote: > On Thu, Nov 27, 2014 at 1:59 PM, Mauro Carvalho Chehab > <mchehab@osg.samsung.com> wrote: >> Em Thu, 27 Nov 2014 13:47:09 -0500 >> Michael Ira Krufky <mkrufky@linuxtv.org> escreveu: >> >>> On Wed, Nov 26, 2014 at 10:26 AM, Mauro Carvalho Chehab >>> <mchehab@osg.samsung.com> wrote: >>>> As reported by smatch: >>>> drivers/media/tuners/tda18271-common.c:176 tda18271_read_extended() warn: if statement not indented >>>> >>>> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> >>>> >>>> diff --git a/drivers/media/tuners/tda18271-common.c b/drivers/media/tuners/tda18271-common.c >>>> index 86e5e3110118..6118203543ea 100644 >>>> --- a/drivers/media/tuners/tda18271-common.c >>>> +++ b/drivers/media/tuners/tda18271-common.c >>>> @@ -173,12 +173,9 @@ int tda18271_read_extended(struct dvb_frontend *fe) >>>> >>>> for (i = 0; i < TDA18271_NUM_REGS; i++) { >>>> /* don't update write-only registers */ >>>> - if ((i != R_EB9) && >>>> - (i != R_EB16) && >>>> - (i != R_EB17) && >>>> - (i != R_EB19) && >>>> - (i != R_EB20)) >>>> - regs[i] = regdump[i]; >>>> + if ((i != R_EB9) && (i != R_EB16) && (i != R_EB17) && >>>> + (i != R_EB19) && (i != R_EB20)) >>>> + regs[i] = regdump[i]; >>>> } >>>> >>>> if (tda18271_debug & DBG_REG) >>>> -- >>>> 1.9.3 >>>> >>> >>> Mauro, >>> >>> I would actually rather NOT merge this patch. This hurts the >>> readability of the code. If applied already, please revert it. >> >> What hurts readability is to not indent regs[i] = regdump[i]; >> >>> >>> Cheers, >>> >>> Mike > > > If the patch were only fixing the indent of "regs[i] = regdump[i];" > then it wouldn't bother me. I don't approve of the whitespace change > in the if statement. > > Please resubmit it as a one-liner that *only* fixes the single bad > indentation of the assignment to regs[i]. switch-case is most suitable for that kind of comparisons. regards Antti
diff --git a/drivers/media/tuners/tda18271-common.c b/drivers/media/tuners/tda18271-common.c index 86e5e3110118..6118203543ea 100644 --- a/drivers/media/tuners/tda18271-common.c +++ b/drivers/media/tuners/tda18271-common.c @@ -173,12 +173,9 @@ int tda18271_read_extended(struct dvb_frontend *fe) for (i = 0; i < TDA18271_NUM_REGS; i++) { /* don't update write-only registers */ - if ((i != R_EB9) && - (i != R_EB16) && - (i != R_EB17) && - (i != R_EB19) && - (i != R_EB20)) - regs[i] = regdump[i]; + if ((i != R_EB9) && (i != R_EB16) && (i != R_EB17) && + (i != R_EB19) && (i != R_EB20)) + regs[i] = regdump[i]; } if (tda18271_debug & DBG_REG)
As reported by smatch: drivers/media/tuners/tda18271-common.c:176 tda18271_read_extended() warn: if statement not indented Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>