On Fri, Apr 08, 2011 at 09:53:33AM -0400, Jeff Layton wrote:
> > diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> > index fb6a2ad..cf8a610 100644
> > --- a/fs/cifs/cifsfs.c
> > +++ b/fs/cifs/cifsfs.c
> > @@ -134,24 +134,20 @@ cifs_read_super(struct super_block *sb, void *data,
> > cifs_sb->bdi.ra_pages = default_backing_dev_info.ra_pages;
> >
> > #ifdef CONFIG_CIFS_DFS_UPCALL
> > - /* copy mount params to sb for use in submounts */
> > - /* BB: should we move this after the mount so we
> > - * do not have to do the copy on failed mounts?
> > - * BB: May be it is better to do simple copy before
> > - * complex operation (mount), and in case of fail
> > - * just exit instead of doing mount and attempting
> > - * undo it if this copy fails?*/
> > + /*
> > + * Copy mount params to sb for use in submounts. Better to do
> > + * the copy here and deal with the error before cleanup gets
> > + * complicated post-mount.
> > + */
> > if (data) {
> > - int len = strlen(data);
> > - cifs_sb->mountdata = kzalloc(len + 1, GFP_KERNEL);
> > + cifs_sb->mountdata = kstrndup(data, PAGE_CACHE_SIZE,
> > + GFP_KERNEL);
> > if (cifs_sb->mountdata == NULL) {
> > bdi_destroy(&cifs_sb->bdi);
> > kfree(sb->s_fs_info);
> > sb->s_fs_info = NULL;
> > return -ENOMEM;
> > }
> > - strncpy(cifs_sb->mountdata, data, len + 1);
> > - cifs_sb->mountdata[len] = '\0';
> > }
> > #endif
> >
>
> I was wrong before, and we should be using PAGE_SIZE here instead of
> PAGE_CACHE_SIZE. On all arches that I know of those are equivalent
> currently, but that might not be the case in the future. It might not
> hurt to change that in these patches though if you're respinning them
> anyway.
Okay, no big deal to do so, will update it before I re-submit the patches
on monday. Thanks for the review!
sean
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
