Re: [Outreachy kernel] [PATCH] staging: xgifb: correct the multiple line dereference to fix coding stye errors

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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]);
+		}
 	}
 }
 
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux