[PATCH v2] staging: xgifb: Rename local variable

[PATCH v2] staging: xgifb: Rename local variable

[Janani Ravichandran]

As one of the global variables and a local variable share the same name,
change the name of the local variable to avoid confusion.

Signed-off-by: Janani Ravichandran <janani.rvchndrn@gmail.com>
---
Changes since v1:
 * Changed the name of the variable to something more meaningful

 drivers/staging/xgifb/XGI_main_26.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index f11cea1..0091d8d 100644
--- a/drivers/staging/xgifb/XGI_main_26.c
+++ b/drivers/staging/xgifb/XGI_main_26.c
@@ -1230,7 +1230,7 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 	unsigned int vtotal = 0;
 	unsigned int drate = 0, hrate = 0;
 	int found_mode = 0;
-	int refresh_rate, search_idx;
+	int local_refresh_rate, search_idx;
 
 	if ((var->vmode & FB_VMODE_MASK) == FB_VMODE_NONINTERLACED) {
 		vtotal = var->upper_margin + var->yres + var->lower_margin
@@ -1268,7 +1268,7 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
 
 	/* Calculation wrong for 1024x600 - force it to 60Hz */
 	if ((var->xres == 1024) && (var->yres == 600))
-		refresh_rate = 60;
+		local_refresh_rate = 60;
 
 	search_idx = 0;
 	while ((XGIbios_mode[search_idx].mode_no != 0) &&
-- 
2.5.0

Re: [Outreachy kernel] [PATCH v2] staging: xgifb: Rename local variable

[Julia Lawall]

On Tue, 9 Feb 2016, Janani Ravichandran wrote:

> As one of the global variables and a local variable share the same name,
> change the name of the local variable to avoid confusion.
> 
> Signed-off-by: Janani Ravichandran <janani.rvchndrn@gmail.com>
> ---
> Changes since v1:
>  * Changed the name of the variable to something more meaningful
> 
>  drivers/staging/xgifb/XGI_main_26.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
> index f11cea1..0091d8d 100644
> --- a/drivers/staging/xgifb/XGI_main_26.c
> +++ b/drivers/staging/xgifb/XGI_main_26.c
> @@ -1230,7 +1230,7 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>  	unsigned int vtotal = 0;
>  	unsigned int drate = 0, hrate = 0;
>  	int found_mode = 0;
> -	int refresh_rate, search_idx;
> +	int local_refresh_rate, search_idx;
>  
>  	if ((var->vmode & FB_VMODE_MASK) == FB_VMODE_NONINTERLACED) {
>  		vtotal = var->upper_margin + var->yres + var->lower_margin
> @@ -1268,7 +1268,7 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>  
>  	/* Calculation wrong for 1024x600 - force it to 60Hz */
>  	if ((var->xres == 1024) && (var->yres == 600))
> -		refresh_rate = 60;
> +		local_refresh_rate = 60;

So this variable is only initialized, but never used?  If that is the 
case, then a better solution would be to drop it.

Or maybe it was not intended to introduce the local variable.  You might 
want to see what you can find with git blame on the line that makes the 
local declaration.

julia

>  
>  	search_idx = 0;
>  	while ((XGIbios_mode[search_idx].mode_no != 0) &&
> -- 
> 2.5.0
> 
> -- 
> 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 outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20160210033729.GA2677%40janani-Inspiron-3521.
> For more options, visit https://groups.google.com/d/optout.
> 

Re: [Outreachy kernel] [PATCH v2] staging: xgifb: Rename local variable

[Sudip Mukherjee]

On Wed, Feb 10, 2016 at 08:29:45AM +0100, Julia Lawall wrote:
> 
> 
> On Tue, 9 Feb 2016, Janani Ravichandran wrote:
> 
> > As one of the global variables and a local variable share the same name,
> > change the name of the local variable to avoid confusion.
> > 
> > Signed-off-by: Janani Ravichandran <janani.rvchndrn@gmail.com>
> > ---
> > Changes since v1:
> >  * Changed the name of the variable to something more meaningful
> > 
> >  drivers/staging/xgifb/XGI_main_26.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
> > index f11cea1..0091d8d 100644
> > --- a/drivers/staging/xgifb/XGI_main_26.c
> > +++ b/drivers/staging/xgifb/XGI_main_26.c
> > @@ -1230,7 +1230,7 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
> >  	unsigned int vtotal = 0;
> >  	unsigned int drate = 0, hrate = 0;
> >  	int found_mode = 0;
> > -	int refresh_rate, search_idx;
> > +	int local_refresh_rate, search_idx;
> >  
> >  	if ((var->vmode & FB_VMODE_MASK) == FB_VMODE_NONINTERLACED) {
> >  		vtotal = var->upper_margin + var->yres + var->lower_margin
> > @@ -1268,7 +1268,7 @@ static int XGIfb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
> >  
> >  	/* Calculation wrong for 1024x600 - force it to 60Hz */
> >  	if ((var->xres == 1024) && (var->yres == 600))
> > -		refresh_rate = 60;
> > +		local_refresh_rate = 60;
> 
> So this variable is only initialized, but never used?  If that is the 
> case, then a better solution would be to drop it.
> 
> Or maybe it was not intended to introduce the local variable.  You might 
> want to see what you can find with git blame on the line that makes the 
> local declaration.

If you remember in the last round of Outreachy also we had a similar
discussion, I will see if I can find out that thread.

regards
sudip

Re: [Outreachy kernel] [PATCH v2] staging: xgifb: Rename local variable

[Janani Ravichandran]

[-- Attachment #1: Type: text/plain, Size: 754 bytes --]

> >
> > So this variable is only initialized, but never used?  If that is the
> > case, then a better solution would be to drop it.
> >
>
I was in fact debating between dropping it and renaming it but decided
on renaming it as I thought it might be used in the future, looking at the
if() check and the comment above it. Not sure if this was a good idea.

> > Or maybe it was not intended to introduce the local variable.  You might
> > want to see what you can find with git blame on the line that makes the
> > local declaration.
>
Sure, I will do that.

>
> If you remember in the last round of Outreachy also we had a similar
> discussion, I will see if I can find out that thread.

I'd love to see it, Sudip.

Thanks,
Janani.

>
> regards
> sudip
>

[-- Attachment #2: Type: text/html, Size: 1673 bytes --]

Re: [Outreachy kernel] [PATCH v2] staging: xgifb: Rename local variable

[Sudip Mukherjee]

On Wed, Feb 10, 2016 at 03:05:05AM -0500, Janani Ravichandran wrote:
> > >
> > > So this variable is only initialized, but never used?  If that is the
> > > case, then a better solution would be to drop it.
> > >
> >
> I was in fact debating between dropping it and renaming it but decided
> on renaming it as I thought it might be used in the future, looking at the
> if() check and the comment above it. Not sure if this was a good idea.
> 
> > > Or maybe it was not intended to introduce the local variable.  You might
> > > want to see what you can find with git blame on the line that makes the
> > > local declaration.
> >
> Sure, I will do that.
> 
> >
> > If you remember in the last round of Outreachy also we had a similar
> > discussion, I will see if I can find out that thread.
> 
> I'd love to see it, Sudip.

https://groups.google.com/forum/#!searchin/outreachy-kernel/Staging$3A$20xgifb$3A$20XGI_main_26$3A$20Remove$20unused$20code/outreachy-kernel/fsXHgjdyx40/Omgja2r3CgAJ

regards
sudip