Re: [PATCH v7] staging: xgifb: correct the multiple line dereference

From: Joe Perches
Date: Tue Feb 28 2017 - 06:13:27 EST


On Tue, 2017-02-28 at 16:15 +0530, Arushi Singhal wrote:
> Error reported by checkpatch.pl as "avoid multiple line dereference".
> Addition of new variables to make the code more readable.

You should probably split this into 2 patches, one for
each file.

What Julia said about finding a better/shorter identifier
than RefreshRateTableIndex is true but I still suggest
something like:

> diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c@@ -221,8 +221,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short ModeIdIndex,
>
> for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
> tempbx; (*i)--) {
> - infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
> - Ext_InfoFlag;
> + infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].Ext_InfoFlag;
> +

I suggest something like:
---
drivers/staging/xgifb/vb_setmode.c | 41 ++++++++++++++++++--------------------
1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/xgifb/vb_setmode.c b/drivers/staging/xgifb/vb_setmode.c
index 7c7c8c8f1df3..daa3318aab41 100644
--- a/drivers/staging/xgifb/vb_setmode.c
+++ b/drivers/staging/xgifb/vb_setmode.c
@@ -172,10 +172,12 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short ModeIdIndex,
struct vb_device_info *pVBInfo)
{
unsigned short tempax, tempbx, resinfo, modeflag, infoflag;
+ const struct XGI_Ext2Struct *ext2;

modeflag = XGI330_EModeIDTable[ModeIdIndex].Ext_ModeFlag;
resinfo = XGI330_EModeIDTable[ModeIdIndex].Ext_RESINFO;
- tempbx = XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID;
+ ext2 = &XGI330_RefIndex[RefreshRateTableIndex];
+ tempbx = ext2[*i].ModeID;
tempax = 0;

if (pVBInfo->VBInfo & SetCRT2ToRAMDAC) {
@@ -219,10 +221,8 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short ModeIdIndex,
return 0;
}

- for (; XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID ==
- tempbx; (*i)--) {
- infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
- Ext_InfoFlag;
+ for (; ext2[*i].ModeID == tempbx; (*i)--) {
+ infoflag = ext2[*i].Ext_InfoFlag;
if (infoflag & tempax)
return 1;

@@ -231,12 +231,9 @@ static unsigned char XGI_AjustCRT2Rate(unsigned short ModeIdIndex,
}

for ((*i) = 0;; (*i)++) {
- infoflag = XGI330_RefIndex[RefreshRateTableIndex + (*i)].
- Ext_InfoFlag;
- if (XGI330_RefIndex[RefreshRateTableIndex + (*i)].ModeID
- != tempbx) {
+ infoflag = ext2[*i].Ext_InfoFlag;
+ if (ext2[*i].ModeID != tempbx)
return 0;
- }

if (infoflag & tempax)
return 1;
@@ -5050,6 +5047,7 @@ unsigned short XGI_GetRatePtrCRT2(struct xgi_hw_device_info *pXGIHWDE,
{
const u8 LCDARefreshIndex[] = {
0x00, 0x00, 0x03, 0x01, 0x01, 0x01, 0x01, 0x00 };
+ const struct XGI_Ext2Struct *ext2;

unsigned short RefreshRateTableIndex, i, index, temp;

@@ -5073,29 +5071,29 @@ unsigned short XGI_GetRatePtrCRT2(struct xgi_hw_device_info *pXGIHWDE,
}

RefreshRateTableIndex = XGI330_EModeIDTable[ModeIdIndex].REFindex;
- ModeNo = XGI330_RefIndex[RefreshRateTableIndex].ModeID;
+ ext2 = &XGI330_RefIndex[RefreshRateTableIndex];
+ ModeNo = ext2->ModeID;
if (pXGIHWDE->jChipType >= XG20) { /* for XG20, XG21, XG27 */
- if ((XGI330_RefIndex[RefreshRateTableIndex].XRes == 800) &&
- (XGI330_RefIndex[RefreshRateTableIndex].YRes == 600)) {
+ if ((ext2->XRes == 800) &&
+ (ext2->YRes == 600)) {
index++;
}
/* do the similar adjustment like XGISearchCRT1Rate() */
- if ((XGI330_RefIndex[RefreshRateTableIndex].XRes == 1024) &&
- (XGI330_RefIndex[RefreshRateTableIndex].YRes == 768)) {
+ if ((ext2->XRes == 1024) &&
+ (ext2->YRes == 768)) {
index++;
}
- if ((XGI330_RefIndex[RefreshRateTableIndex].XRes == 1280) &&
- (XGI330_RefIndex[RefreshRateTableIndex].YRes == 1024)) {
+ if ((ext2->XRes == 1280) &&
+ (ext2->YRes == 1024)) {
index++;
}
}

i = 0;
do {
- if (XGI330_RefIndex[RefreshRateTableIndex + i].
- ModeID != ModeNo)
+ if (ext2[i].ModeID != ModeNo)
break;
- temp = XGI330_RefIndex[RefreshRateTableIndex + i].Ext_InfoFlag;
+ temp = ext2[i].Ext_InfoFlag;
temp &= ModeTypeMask;
if (temp < pVBInfo->ModeType)
break;
@@ -5105,8 +5103,7 @@ unsigned short XGI_GetRatePtrCRT2(struct xgi_hw_device_info *pXGIHWDE,
} while (index != 0xFFFF);
if (!(pVBInfo->VBInfo & SetCRT2ToRAMDAC)) {
if (pVBInfo->VBInfo & SetInSlaveMode) {
- temp = XGI330_RefIndex[RefreshRateTableIndex + i - 1].
- Ext_InfoFlag;
+ temp = ext2[i - 1].Ext_InfoFlag;
if (temp & InterlaceMode)
i++;
}