Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-21 Thread Dan Carpenter
That looks nice. Thank you. Sorry for the headaches. regards, dan carpenter

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-21 Thread Dan Carpenter
That looks nice. Thank you. Sorry for the headaches. regards, dan carpenter

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-21 Thread Dan Carpenter
On Tue, Mar 20, 2018 at 07:32:32PM +0530, Pratik Jain wrote: > Refactored the function `XGIfb_search_refresh_rate` by removing a level > of `if...else` block nesting. Removed unnecessary parantheses. > > Signed-off-by: Pratik Jain > --- >

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-21 Thread Dan Carpenter
On Tue, Mar 20, 2018 at 07:32:32PM +0530, Pratik Jain wrote: > Refactored the function `XGIfb_search_refresh_rate` by removing a level > of `if...else` block nesting. Removed unnecessary parantheses. > > Signed-off-by: Pratik Jain > --- > drivers/staging/xgifb/XGI_main_26.c | 59 >

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-20 Thread Dan Carpenter
On Tue, Mar 20, 2018 at 02:05:49PM +0530, Pratik Jain wrote: > Refactored the function `XGIfb_search_refresh_rate` by removing a level > of `if...else` block nesting. Removed unnecessary parantheses. > > Signed-off-by: Pratik Jain > --- >

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-20 Thread Dan Carpenter
On Tue, Mar 20, 2018 at 02:05:49PM +0530, Pratik Jain wrote: > Refactored the function `XGIfb_search_refresh_rate` by removing a level > of `if...else` block nesting. Removed unnecessary parantheses. > > Signed-off-by: Pratik Jain > --- > drivers/staging/xgifb/XGI_main_26.c | 63 >

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-20 Thread Pratik Jain
You got a valid point about `++i` and `i++`. But I still feel that it is less complicated than previous one. I am explicitly saying(in first loop) that we are skipping some values to get to a specific index. Apart from that, we can avoid unncessary wrapping and indentation. Logic becomes much more

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-20 Thread Pratik Jain
You got a valid point about `++i` and `i++`. But I still feel that it is less complicated than previous one. I am explicitly saying(in first loop) that we are skipping some values to get to a specific index. Apart from that, we can avoid unncessary wrapping and indentation. Logic becomes much more

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-20 Thread Dan Carpenter
I'm trying to review this, but I feel like this makes it slightly more complicated for no reason. Why break it up into two loops? > - i++; > + ++i; These are equivalent, so you should default to accepting the original author's style. Otherwise the next person to touch

Re: [PATCH] Staging: xgifb: XGI_main_26.c: Refactored the function

2018-03-20 Thread Dan Carpenter
I'm trying to review this, but I feel like this makes it slightly more complicated for no reason. Why break it up into two loops? > - i++; > + ++i; These are equivalent, so you should default to accepting the original author's style. Otherwise the next person to touch