Re: [patch] fix BUG: in fw_realloc_buffer

[Andrew Morton <akpm@osdl.org>]

Jeff Moyer <jmoyer@redhat.com> wrote:
>
> akpm> A little bit neater this way, I think?
> 
> > --- devel/drivers/base/firmware_class.c~firmware-fix-bug-in-fw_realloc_buffer	2006-02-13 14:45:52.000000000 -0800
> > +++ devel-akpm/drivers/base/firmware_class.c	2006-02-13 14:52:05.000000000 -0800
> > @@ -211,18 +211,20 @@ static int
> >  fw_realloc_buffer(struct firmware_priv *fw_priv, int min_size)
> >  {
> >  	u8 *new_data;
> > +	int new_size = fw_priv->alloc_size;
> 
> >  	if (min_size <= fw_priv->alloc_size)
> >  		return 0;
> 
> > -	new_data = vmalloc(fw_priv->alloc_size + PAGE_SIZE);
> > +	new_size = ALIGN(min_size, PAGE_SIZE);
> > +	new_data = vmalloc(new_size);
> >  	if (!new_data) {
> >  		printk(KERN_ERR "%s: unable to alloc buffer\n", __FUNCTION__);
> >  		/* Make sure that we don't keep incomplete data */
> >  		fw_load_abort(fw_priv);
> >  		return -ENOMEM;
> >  	}
> > -	fw_priv->alloc_size += PAGE_SIZE;
> > +	fw_priv->alloc_size = new_size;
> >  	if (fw_priv->fw->data) {
> >  		memcpy(new_data, fw_priv->fw->data, fw_priv->fw->size);
> >  		vfree(fw_priv->fw->data);
> > _
> 
> Well, I wasn't sure that you would only need to increase by a PAGE.  If you
> only need to account for page_size + alignment, then yes, this is better.
> It simply wasn't clear to me that this is how we are called.  If I'm not
> mistaken, this is the write routine for a file in sysfs.  If that is the
> case, why should we assume that writes are broken up into PAGE_SIZE chunks?
> 

hm?  The code's equivanent, I think.  ->alloc_size is always a multiple of
PAGE_SIZE and the ALIGN makes new_size the next multiple of PAGE_SIZE which
is >= min_size.