All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/1] fs/cifs/dir.c: use correct path separators
@ 2016-09-07 13:45 Aurelien Aptel
       [not found] ` <1473255952-27579-1-git-send-email-aaptel-IBi9RG/b67k@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Aurelien Aptel @ 2016-09-07 13:45 UTC (permalink / raw)
  To: linux-cifs-u79uwXL29TY76Z2rM5mHXA; +Cc: Aurelien Aptel

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;
 	}
 
 	if (dfsplen) {
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v1 1/1] fs/cifs/dir.c: use correct path separators
       [not found] ` <1473255952-27579-1-git-send-email-aaptel-IBi9RG/b67k@public.gmane.org>
@ 2016-09-12 18:36   ` Sachin Prabhu
       [not found]     ` <1473705389.29354.20.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Sachin Prabhu @ 2016-09-12 18:36 UTC (permalink / raw)
  To: Aurelien Aptel, linux-cifs-u79uwXL29TY76Z2rM5mHXA

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) {

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v1 1/1] fs/cifs/dir.c: use correct path separators
       [not found]     ` <1473705389.29354.20.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2016-09-17  7:54       ` Aurélien Aptel
  0 siblings, 0 replies; 3+ messages in thread
From: Aurélien Aptel @ 2016-09-17  7:54 UTC (permalink / raw)
  To: Sachin Prabhu, linux-cifs-u79uwXL29TY76Z2rM5mHXA

Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> writes:
> We have an inline helper convert_delimiter() which already does this.
> The block for dfsplen also seems to repeat the code. 

Correct except for a small detail: the inline helper does the conversion
until the end of the string. Here we need to stop in the middle.

> Can we use the  helper for the complete patch instead when the
> full_path variable is available?

We do not want to mess with unix path that might contain a valid
backslash inside a path component.

> 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.

Agreed. Thanks for the feedback, Sachin.
I'll send an updated patch for review.

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-09-17  7:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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
     [not found]     ` <1473705389.29354.20.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2016-09-17  7:54       ` Aurélien Aptel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.