Re: [Outreachy kernel] [PATCH] Staging: xgifb: XGI_main_26: Remove unused code

[Sudip Mukherjee <sudipm.mukherjee@gmail.com>]

On Mon, Oct 12, 2015 at 03:28:33PM +0530, Shivani Bhardwaj wrote:
> On Mon, Oct 12, 2015 at 2:52 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> > On Mon, 12 Oct 2015, Shivani Bhardwaj wrote:
> >
> >> Remove unused variable and its occurences as it is not used anywhere in
> >> the code.
> >> Issue found using coccinelle.
> >>
> >> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com>
> >> ---
> >>
> >> -     /* Calculation wrong for 1024x600 - force it to 60Hz */
> >> -     if ((var->xres == 1024) && (var->yres == 600))
> >> -             refresh_rate = 60;
> >
> > Could it be supposed to be xgifb_info->refresh_rate = 60; ?
> >
> > julia
> >
> 
> Won't that be a part of structure whose object is xgifb_info? This is
> any normal int type. Plus, it doesn't give any errors when removed, it
> should have if it were xgifb_info's member. Please correct me if I  am
> wrong.

From C syntax point of view your patch is correct. But looking at the
comment mentioned there this could have been:

diff --git a/drivers/staging/xgifb/XGI_main_26.c b/drivers/staging/xgifb/XGI_main_26.c
index 5a6251a4..0436dbe 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 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;
+		xgifb_info->refresh_rate = 60;
 
 	search_idx = 0;
 	while ((XGIbios_mode[search_idx].mode_no != 0) &&

---
Maybe this was the actual intention of the developer as there is also
another global static variable named as refresh_rate which is having the
refresh frequency and saving the Hz value in xgifb_info->refresh_rate.
But that part is in the probe function and the probe will be called
before fb_check_var callback.
Anyways, this was just a thought, please donot send v2 based on this.
Let Greg see it first.

regards
sudip