On Wed, Jul 23, 2014 at 12:52:37PM +0200, Matthias Beyer wrote: > On 23-07-2014 13:48:11, Dan Carpenter wrote: > > On Sun, Jul 20, 2014 at 03:14:10PM +0200, Matthias Beyer wrote: > > > This patch outsources a chunk of code into an own function. It also > > > refactors the variable names which are used within this function. > > > > > > The function name may be not appropriate. > > > > > > Signed-off-by: Matthias Beyer <mail@xxxxxxxxxxxxxxxx> > > > --- > > > drivers/staging/bcm/nvm.c | 70 ++++++++++++++++++++++++++++++++--------------- > > > 1 file changed, 48 insertions(+), 22 deletions(-) > > > > > > diff --git a/drivers/staging/bcm/nvm.c b/drivers/staging/bcm/nvm.c > > > index 76c86eb..4aa195c 100644 > > > --- a/drivers/staging/bcm/nvm.c > > > +++ b/drivers/staging/bcm/nvm.c > > > @@ -1033,6 +1033,44 @@ static ULONG BcmFlashUnProtectBlock(struct bcm_mini_adapter *Adapter, unsigned i > > > return ulStatus; > > > } > > > > > > +static int bulk_read_complete_sector(struct bcm_mini_adapter *ad, > > > + UCHAR read_bk[], > > > + PCHAR tmpbuff, > > > + unsigned int offset, > > > + unsigned int partoff, > > > + unsigned int i) > > > > "i" should just be a local variable here. > > > > Could you send a follow on patch to clean that up? > > > > Also this code has that disease that every variable is "unsigned int". > > It can't go higher than MAX_SECTOR_SIZE. It should just be "int" and > > the same for "j" because that is a number between 0-15. > > > > regards, > > dan carpenter > > > > sure I can, but the variable which is named 'i' in the argument list > is passed from the calling function where it is an unsigned int! We shouldn't be passing it. > > Should I fix this up, too? There is so much to fix up in this driver. :P I wouldn't worry about little things too much at this point. regards, dan carpenter _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
