Skip to contentSkip to conversation

staging: xgifb: vb_setmode: fix multiple line dereference and indentations

PATCH In review Kimberly Brown 2 participants 6 messages Oct 28, 2018 – Oct 29, 2018 Staging

lore
Share & tools
lore

Kimberly Brown

author
#0 lore raw

cover staging: xgifb: vb_setmode: fix multiple line dereference and indentations

This patchset fixes a multiple line dereference and adjusts indentations to comply with the Linux kernel coding style.

Kimberly Brown (2):
staging: xgifb: vb_setmode: fix multiple line dereference
staging: xgifb: vb_setmode: adjust descendant indentation to right

 drivers/staging/xgifb/vb_setmode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
Signature · 1 line
2.17.1
author
#1 lore raw

PATCH 1/2 staging: xgifb: vb_setmode: fix multiple line dereference

Fix multiple line dereference by adding a new line and adjusting indentations. This change improves code readability. Issue found by checkpatch.

Signed-off-by Kimberly Brown <[email protected]>
---
 drivers/staging/xgifb/vb_setmode.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/staging/xgifb/vb_setmode.c +3 -2
index 1fa0dc66406e..62b45a30e0f1 100644
@@ -656,8 +656,9 @@ static void XGI_UpdateXG21CRTC(unsigned short ModeNo,
 	    (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
 						      RES640x480x60))
 		index = 12;
-	else if (ModeNo == 0x2E && (XGI330_RefIndex[RefreshRateTableIndex].
-				Ext_CRT1CRTC == RES640x480x72))
+	else if (ModeNo == 0x2E &&
+		 (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
+								RES640x480x72))
 		index = 13;
 	else if (ModeNo == 0x2F)
 		index = 14;
Signature · 1 line
2.17.1
author
#2 lore raw

PATCH 2/2 staging: xgifb: vb_setmode: adjust descendant indentation to right

In reply to Kimberly Brown

Adjust descendant indentation so that it is placed substantially to the right. This change improves code readability and complies with the Linux kernel coding style.

Signed-off-by Kimberly Brown <[email protected]>
---
 drivers/staging/xgifb/vb_setmode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/xgifb/vb_setmode.c +1 -1
index 62b45a30e0f1..3782f8641bf2 100644
@@ -654,7 +654,7 @@ static void XGI_UpdateXG21CRTC(unsigned short ModeNo,
 	xgifb_reg_and(pVBInfo->P3d4, 0x11, 0x7F); /* Unlock CR0~7 */
 	if (ModeNo == 0x2E &&
 	    (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
-						      RES640x480x60))
+								RES640x480x60))
 		index = 12;
 	else if (ModeNo == 0x2E &&
 		 (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
Signature · 1 line
2.17.1
#3 lore raw

Re: [Outreachy kernel] [PATCH 2/2] staging: xgifb: vb_setmode: adjust descendant indentation to right

Quoted 14 lines · Kimberly Brown

Adjust descendant indentation so that it is placed substantially to the right. This change improves code readability and complies with the Linux kernel coding style.

Signed-off-by Kimberly Brown <[email protected]>
---
 drivers/staging/xgifb/vb_setmode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/xgifb/vb_setmode.c +0 -0
index 62b45a30e0f1..3782f8641bf2 100644

drivers/staging/xgifb/vb_setmode.c static void XGI_UpdateXG21CRTC(unsigned short ModeNo, · line 657

Quoted 5 lines
drivers/staging/xgifb/vb_setmode.c +1 -1
@@ -654,7 +654,7 @@ static void XGI_UpdateXG21CRTC(unsigned short ModeNo,
 	xgifb_reg_and(pVBInfo->P3d4, 0x11, 0x7F); /* Unlock CR0~7 */
 	if (ModeNo == 0x2E &&
 	    (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
-						      RES640x480x60))
+								RES640x480x60))

The variable RefreshRateTableIndex is really long,and uses camel case, which is also not the kernel style. If there is a shorter version that would be acceptable, then perhaps the value to the right of the == could be on the same line, thus solving two problems at once.

julia

Quoted 13 lines
drivers/staging/xgifb/vb_setmode.c +0 -1
 		index = 12;
 	else if (ModeNo == 0x2E &&
 		 (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
--

2.17.1

--
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 [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/a85f97b40dabc0f2a978d0928af6bba05b9724e5.1540749179.git.kimbrownkd%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

author
#4 lore raw

Re: [Outreachy kernel] [PATCH 2/2] staging: xgifb: vb_setmode: adjust descendant indentation to right

Quoted 13 lines · Julia Lawall
Quoted 9 lines · Kimberly Brown

Adjust descendant indentation so that it is placed substantially to the right. This change improves code readability and complies with the Linux kernel coding style.

Signed-off-by Kimberly Brown <[email protected]>
---
 drivers/staging/xgifb/vb_setmode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

drivers/staging/xgifb/vb_setmode.c static void XGI_UpdateXG21CRTC(unsigned short ModeNo, · line 657

Quoted 10 lines
drivers/staging/xgifb/vb_setmode.c +1 -1
index 62b45a30e0f1..3782f8641bf2 100644
@@ -654,7 +654,7 @@ static void XGI_UpdateXG21CRTC(unsigned short ModeNo,
 	xgifb_reg_and(pVBInfo->P3d4, 0x11, 0x7F); /* Unlock CR0~7 */
 	if (ModeNo == 0x2E &&
 	    (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
-						      RES640x480x60))
+								RES640x480x60))

The variable RefreshRateTableIndex is really long,and uses camel case, which is also not the kernel style. If there is a shorter version that would be acceptable, then perhaps the value to the right of the == could be on the same line, thus solving two problems at once.

Julia,

Good point! I propose 'ref_index' to replace 'RefreshRateTableIndex'. I chose this based on the 'RefreshRateTableIndex' assignment on line 5096:

RefreshRateTableIndex = XGI330_EModeIDTable[ModeIdIndex].REFindex;

Do you have any objections to 'ref_index'?

Thanks!

Quoted 16 lines

julia

Quoted 13 lines
drivers/staging/xgifb/vb_setmode.c +0 -1
 		index = 12;
 	else if (ModeNo == 0x2E &&
 		 (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
--

2.17.1

--
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 [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/a85f97b40dabc0f2a978d0928af6bba05b9724e5.1540749179.git.kimbrownkd%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

#5 lore raw

Re: [Outreachy kernel] [PATCH 2/2] staging: xgifb: vb_setmode: adjust descendant indentation to right

Quoted 30 lines · Kimberly Brown
Quoted 13 lines · Julia Lawall
Quoted 9 lines · Kimberly Brown

Adjust descendant indentation so that it is placed substantially to the right. This change improves code readability and complies with the Linux kernel coding style.

Signed-off-by Kimberly Brown <[email protected]>
---
 drivers/staging/xgifb/vb_setmode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 15 lines

drivers/staging/xgifb/vb_setmode.c static void XGI_UpdateXG21CRTC(unsigned short ModeNo, · line 657

Quoted 10 lines
drivers/staging/xgifb/vb_setmode.c +1 -1
index 62b45a30e0f1..3782f8641bf2 100644
@@ -654,7 +654,7 @@ static void XGI_UpdateXG21CRTC(unsigned short ModeNo,
 	xgifb_reg_and(pVBInfo->P3d4, 0x11, 0x7F); /* Unlock CR0~7 */
 	if (ModeNo == 0x2E &&
 	    (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
-						      RES640x480x60))
+								RES640x480x60))

The variable RefreshRateTableIndex is really long,and uses camel case, which is also not the kernel style. If there is a shorter version that would be acceptable, then perhaps the value to the right of the == could be on the same line, thus solving two problems at once.

Quoted 8 lines

Julia,

Good point! I propose 'ref_index' to replace 'RefreshRateTableIndex'. I chose this based on the 'RefreshRateTableIndex' assignment on line 5096:

RefreshRateTableIndex = XGI330_EModeIDTable[ModeIdIndex].REFindex;

Do you have any objections to 'ref_index'?

Intuitively, I would have suggested rate_index, but I can see your reasoning in this case.

julia

Quoted 27 lines

Thanks!

Quoted 16 lines

julia

Quoted 13 lines
drivers/staging/xgifb/vb_setmode.c +0 -1
 		index = 12;
 	else if (ModeNo == 0x2E &&
 		 (XGI330_RefIndex[RefreshRateTableIndex].Ext_CRT1CRTC ==
--

2.17.1

--
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 [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/a85f97b40dabc0f2a978d0928af6bba05b9724e5.1540749179.git.kimbrownkd%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

--
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 [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20181029022658.GA158541%40ubu-Virtual-Machine.
For more options, visit https://groups.google.com/d/optout.