Re: [Outreachy kernel] [PATCH] staging: xgifb: correct the multiple line dereference to fix coding stye errors
From: Joe Perches
Date: Tue Feb 21 2017 - 23:34:05 EST
On Wed, 2017-02-22 at 09:46 +0530, Arushi Singhal wrote:
> On Wed, Feb 22, 2017 at 12:12 AM, Joe Perches <joe@xxxxxxxxxxx> wrote:
>
> > On Tue, 2017-02-21 at 18:40 +0100, Julia Lawall wrote:
> > >
> > > On Tue, 21 Feb 2017, Arushi Singhal wrote:
> > >
> > > > Error was reported by checkpatch.pl as
> > > > WARNING: Avoid multiple line dereference...
> > > > if there is boolean operator then it is fixed by Splitting line at
> > > > boolean operator.
> > >
> > > This is massively execeeding the 80 character boundary, and not for
> > > something trivial like a string. Maybe the code can be reorganized in
> > > some other way.
> >
> > The easiest way to do that is to change the test above it
> > to reduce indentation from
> >
> > if (xgifb_info->display2 == XGIFB_DISP_TV &&
> > xgifb_info->hasVB == HASVB_301) {
> > [code...];
> > }
> > }
> >
> > to
> >
> > if (xgifb_info->display2 != XGIFB_DISP_TV ||
> > xgifb_in
> > fo->hasVB != HASVB_301)
> > return;
> >
> > [code...];
> > }
> >
> >
>
> Hi
> So basically you are saying that cut one word and put the other part of the
> word in the next line.
No, evolution is a horrible email client
that wraps lines oddly sometimes and sends
out different content than it displays in
its on-screen editor.
What I am saying is the block after the test
can be unindented one level by inverting the
test and using return.
It should be:
if (xgifb_info->display2 != XGIFB_DISP_TV ||
xgifb_in> > fo->hasVB != HASVB_301)
return;
[code...];
I sent you a separate attachment privately a
few hours ago but I'll attach it here too.
drivers/staging/xgifb/XGI_main_26.c | 256 +++++++++++++-----------------------
1 file changed, 92 insertions(+), 164 deletions(-)
diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index 6930f7eb741b..0df7fb2b8892 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -731,179 +731,107 @@ static void XGIfb_post_setmode(struct xgifb_video_info *xgifb_info)
xgifb_reg_and(XGISR, IND_SIS_RAMDAC_CONTROL, ~0x04);
- if (xgifb_info->display2 == XGIFB_DISP_TV &&
- xgifb_info->hasVB == HASVB_301) {
- reg = xgifb_reg_get(XGIPART4, 0x01);
+ if (xgifb_info->display2 != XGIFB_DISP_TV ||
+ xgifb_info->hasVB != HASVB_301)
+ return;
- if (reg < 0xB0) { /* Set filter for XGI301 */
- int filter_tb;
+ reg = xgifb_reg_get(XGIPART4, 0x01);
+ if (reg < 0xB0) { /* Set filter for XGI301 */
+ int filter_tb;
- switch (xgifb_info->video_width) {
- case 320:
- filter_tb = (xgifb_info->TV_type ==
- TVMODE_NTSC) ? 4 : 12;
- break;
- case 640:
- filter_tb = (xgifb_info->TV_type ==
- TVMODE_NTSC) ? 5 : 13;
- break;
- case 720:
- filter_tb = (xgifb_info->TV_type ==
- TVMODE_NTSC) ? 6 : 14;
- break;
- case 800:
- filter_tb = (xgifb_info->TV_type ==
- TVMODE_NTSC) ? 7 : 15;
- break;
- default:
- filter_tb = 0;
- filter = -1;
- break;
- }
- xgifb_reg_or(XGIPART1,
- SIS_CRT2_WENABLE_315,
- 0x01);
+ switch (xgifb_info->video_width) {
+ case 320:
+ filter_tb = xgifb_info->TV_type == TVMODE_NTSC ? 4 : 12;
+ break;
+ case 640:
+ filter_tb = xgifb_info->TV_type == TVMODE_NTSC ? 5 : 13;
+ break;
+ case 720:
+ filter_tb = xgifb_info->TV_type == TVMODE_NTSC ? 6 : 14;
+ break;
+ case 800:
+ filter_tb = xgifb_info->TV_type == TVMODE_NTSC ? 7 : 15;
+ break;
+ default:
+ filter_tb = 0;
+ filter = -1;
+ break;
+ }
+ xgifb_reg_or(XGIPART1, SIS_CRT2_WENABLE_315, 0x01);
- if (xgifb_info->TV_type == TVMODE_NTSC) {
- xgifb_reg_and(XGIPART2, 0x3a, 0x1f);
-
- if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
- xgifb_reg_and(XGIPART2, 0x30, 0xdf);
-
- } else if (xgifb_info->TV_plug
- == TVPLUG_COMPOSITE) {
- xgifb_reg_or(XGIPART2, 0x30, 0x20);
-
- switch (xgifb_info->video_width) {
- case 640:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xEB);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0x04);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x25);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x18);
- break;
- case 720:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xEE);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0x0C);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x22);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x08);
- break;
- case 800:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xEB);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0x15);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x25);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0xF6);
- break;
- }
- }
+ if (xgifb_info->TV_type == TVMODE_NTSC) {
+ xgifb_reg_and(XGIPART2, 0x3a, 0x1f);
- } else if (xgifb_info->TV_type == TVMODE_PAL) {
- xgifb_reg_and(XGIPART2, 0x3A, 0x1F);
-
- if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
- xgifb_reg_and(XGIPART2, 0x30, 0xDF);
-
- } else if (xgifb_info->TV_plug
- == TVPLUG_COMPOSITE) {
- xgifb_reg_or(XGIPART2, 0x30, 0x20);
-
- switch (xgifb_info->video_width) {
- case 640:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xF1);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0xF7);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x1F);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x32);
- break;
- case 720:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xF3);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0x00);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x1D);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x20);
- break;
- case 800:
- xgifb_reg_set(XGIPART2,
- 0x35,
- 0xFC);
- xgifb_reg_set(XGIPART2,
- 0x36,
- 0xFB);
- xgifb_reg_set(XGIPART2,
- 0x37,
- 0x14);
- xgifb_reg_set(XGIPART2,
- 0x38,
- 0x2A);
- break;
- }
+ if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
+ xgifb_reg_and(XGIPART2, 0x30, 0xdf);
+
+ } else if (xgifb_info->TV_plug == TVPLUG_COMPOSITE) {
+ xgifb_reg_or(XGIPART2, 0x30, 0x20);
+
+ switch (xgifb_info->video_width) {
+ case 640:
+ xgifb_reg_set(XGIPART2, 0x35, 0xEB);
+ xgifb_reg_set(XGIPART2, 0x36, 0x04);
+ xgifb_reg_set(XGIPART2, 0x37, 0x25);
+ xgifb_reg_set(XGIPART2, 0x38, 0x18);
+ break;
+ case 720:
+ xgifb_reg_set(XGIPART2, 0x35, 0xEE);
+ xgifb_reg_set(XGIPART2, 0x36, 0x0C);
+ xgifb_reg_set(XGIPART2, 0x37, 0x22);
+ xgifb_reg_set(XGIPART2, 0x38, 0x08);
+ break;
+ case 800:
+ xgifb_reg_set(XGIPART2, 0x35, 0xEB);
+ xgifb_reg_set(XGIPART2, 0x36, 0x15);
+ xgifb_reg_set(XGIPART2, 0x37, 0x25);
+ xgifb_reg_set(XGIPART2, 0x38, 0xF6);
+ break;
}
}
- if ((filter >= 0) && (filter <= 7)) {
- pr_debug("FilterTable[%d]-%d: %*ph\n",
- filter_tb, filter,
- 4, XGI_TV_filter[filter_tb].
- filter[filter]);
- xgifb_reg_set(
- XGIPART2,
- 0x35,
- (XGI_TV_filter[filter_tb].
- filter[filter][0]));
- xgifb_reg_set(
- XGIPART2,
- 0x36,
- (XGI_TV_filter[filter_tb].
- filter[filter][1]));
- xgifb_reg_set(
- XGIPART2,
- 0x37,
- (XGI_TV_filter[filter_tb].
- filter[filter][2]));
- xgifb_reg_set(
- XGIPART2,
- 0x38,
- (XGI_TV_filter[filter_tb].
- filter[filter][3]));
+ } else if (xgifb_info->TV_type == TVMODE_PAL) {
+ xgifb_reg_and(XGIPART2, 0x3A, 0x1F);
+
+ if (xgifb_info->TV_plug == TVPLUG_SVIDEO) {
+ xgifb_reg_and(XGIPART2, 0x30, 0xDF);
+
+ } else if (xgifb_info->TV_plug == TVPLUG_COMPOSITE) {
+ xgifb_reg_or(XGIPART2, 0x30, 0x20);
+
+ switch (xgifb_info->video_width) {
+ case 640:
+ xgifb_reg_set(XGIPART2, 0x35, 0xF1);
+ xgifb_reg_set(XGIPART2, 0x36, 0xF7);
+ xgifb_reg_set(XGIPART2, 0x37, 0x1F);
+ xgifb_reg_set(XGIPART2, 0x38, 0x32);
+ break;
+ case 720:
+ xgifb_reg_set(XGIPART2, 0x35, 0xF3);
+ xgifb_reg_set(XGIPART2, 0x36, 0x00);
+ xgifb_reg_set(XGIPART2, 0x37, 0x1D);
+ xgifb_reg_set(XGIPART2, 0x38, 0x20);
+ break;
+ case 800:
+ xgifb_reg_set(XGIPART2, 0x35, 0xFC);
+ xgifb_reg_set(XGIPART2, 0x36, 0xFB);
+ xgifb_reg_set(XGIPART2, 0x37, 0x14);
+ xgifb_reg_set(XGIPART2, 0x38, 0x2A);
+ break;
+ }
}
}
+
+ if (filter >= 0 && filter <= 7) {
+ const u8 *f = XGI_TV_filter[filter_tb].filter[filter];
+
+ pr_debug("FilterTable[%d]-%d: %*ph\n",
+ filter_tb, filter, 4, f);
+ xgifb_reg_set(XGIPART2, 0x35, f[0]);
+ xgifb_reg_set(XGIPART2, 0x36, f[1]);
+ xgifb_reg_set(XGIPART2, 0x37, f[2]);
+ xgifb_reg_set(XGIPART2, 0x38, f[3]);
+ }
}
}