Looks fine.
On Wed, Mar 30, 2011 at 8:07 AM, Sean Finney <seanius@xxxxxxxxxxx> wrote:
>
> A relatively minor nit, but also clarified the "consensus" from the
> preceding comments that it is in fact better to try for the kstrdup
> early and cleanup while cleaning up is still a simple thing to do.
>
> Signed-off-by: Sean Finney <seanius@xxxxxxxxxxx>
> ---
> fs/cifs/cifsfs.c | 17 ++++++-----------
> 1 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index fb6a2ad..e938bf0 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -134,24 +134,19 @@ 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 = kstrdup(data, 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
>
> --
> 1.7.2.3
>
> --
> 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
--
Thanks,
Steve
--
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
