Linux CIFS filesystem development
 help / color / mirror / Atom feed
From: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
To: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>,
	linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH v1 1/1] fs/cifs/dir.c: use correct path separators
Date: Mon, 12 Sep 2016 19:36:29 +0100	[thread overview]
Message-ID: <1473705389.29354.20.camel@redhat.com> (raw)
In-Reply-To: <1473255952-27579-1-git-send-email-aaptel-IBi9RG/b67k@public.gmane.org>

Thanks Aurelien. Comments below.

On Wed, 2016-09-07 at 15:45 +0200, Aurelien Aptel wrote:
> When a prefix path needs to be added, current code hardcodes the
> initial
> path separator to backslash. Use CIFS_DIR_SEP instead and properly
> convert path separator in the prefix path.
> 
> CIFS_MOUNT_USE_PREFIX_PATH is rarely needed (Windows shares with very
> specific ACL or buggy mount call AFAIK) which explains why the
> backsplash wasn't a big issue.
> 
> Signed-off-by: Aurelien Aptel <aaptel-IBi9RG/b67k@public.gmane.org>
> ---
>  fs/cifs/dir.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index 4716c54..c85a8bb 100644
> --- a/fs/cifs/dir.c
> +++ b/fs/cifs/dir.c
> @@ -160,13 +160,15 @@ cifs_bp_rename_retry:
>  
>  	if (pplen) {
>  		int i;
> +		char oldsep = dirsep == '/' ? '\\' : '/';
>  
>  		cifs_dbg(FYI, "using cifs_sb prepath <%s>\n",
> cifs_sb->prepath);
> +
> +		full_path[dfsplen] = dirsep;
>  		memcpy(full_path+dfsplen+1, cifs_sb->prepath, pplen-
> 1);
> -		full_path[dfsplen] = '\\';
> -		for (i = 0; i < pplen-1; i++)
> -			if (full_path[dfsplen+1+i] == '/')
> -				full_path[dfsplen+1+i] =
> CIFS_DIR_SEP(cifs_sb);
> +		for (i = 1; i < pplen; i++)
> +			if (full_path[dfsplen+i] == oldsep)
> +				full_path[dfsplen+i] = dirsep;

We have an inline helper convert_delimiter() which already does this.
The block for dfsplen also seems to repeat the code. 

Can we use the  helper for the complete patch instead when the
full_path variable is available?
I think the code here for copying the treename and the prefixpath name
can also be cleaned to make it easier to read ie. move the treeName
copy before we copy over the prefix path.

Sachin Prabhu

>  	}
>  
>  	if (dfsplen) {

  parent reply	other threads:[~2016-09-12 18:36 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-07 13:45 [PATCH v1 1/1] fs/cifs/dir.c: use correct path separators Aurelien Aptel
     [not found] ` <1473255952-27579-1-git-send-email-aaptel-IBi9RG/b67k@public.gmane.org>
2016-09-12 18:36   ` Sachin Prabhu [this message]
     [not found]     ` <1473705389.29354.20.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-09-17  7:54       ` Aurélien Aptel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1473705389.29354.20.camel@redhat.com \
    --to=sprabhu-h+wxahxf7alqt0dzr+alfa@public.gmane.org \
    --cc=aaptel-IBi9RG/b67k@public.gmane.org \
    --cc=linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox