* [PATCH v1] sunrpc/cache: convert to use string_escape_str()
@ 2014-11-28 15:50 Andy Shevchenko
2014-12-08 22:38 ` J. Bruce Fields
0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2014-11-28 15:50 UTC (permalink / raw)
To: linux-nfs, J. Bruce Fields, David S. Miller; +Cc: Andy Shevchenko
There is nice kernel helper to escape a given strings by provided rules. Let's
use it instead of custom approach.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
net/sunrpc/cache.c | 27 ++++++---------------------
1 file changed, 6 insertions(+), 21 deletions(-)
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 0663621..5cf60a4 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -20,6 +20,7 @@
#include <linux/list.h>
#include <linux/module.h>
#include <linux/ctype.h>
+#include <linux/string_helpers.h>
#include <asm/uaccess.h>
#include <linux/poll.h>
#include <linux/seq_file.h>
@@ -1067,32 +1068,16 @@ void qword_add(char **bpp, int *lp, char *str)
{
char *bp = *bpp;
int len = *lp;
- char c;
+ int ret;
if (len < 0) return;
- while ((c=*str++) && len)
- switch(c) {
- case ' ':
- case '\t':
- case '\n':
- case '\\':
- if (len >= 4) {
- *bp++ = '\\';
- *bp++ = '0' + ((c & 0300)>>6);
- *bp++ = '0' + ((c & 0070)>>3);
- *bp++ = '0' + ((c & 0007)>>0);
- }
- len -= 4;
- break;
- default:
- *bp++ = c;
- len--;
- }
- if (c || len <1) len = -1;
+ ret = string_escape_str(str, &bp, len, ESCAPE_OCTAL, "\\ \n\t");
+ if (ret < 0 || ret == len)
+ len = -1;
else {
*bp++ = ' ';
- len--;
+ len -= ret - 1;
}
*bpp = bp;
*lp = len;
--
2.1.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v1] sunrpc/cache: convert to use string_escape_str()
2014-11-28 15:50 [PATCH v1] sunrpc/cache: convert to use string_escape_str() Andy Shevchenko
@ 2014-12-08 22:38 ` J. Bruce Fields
2014-12-09 9:24 ` Andy Shevchenko
0 siblings, 1 reply; 4+ messages in thread
From: J. Bruce Fields @ 2014-12-08 22:38 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-nfs, David S. Miller
On Fri, Nov 28, 2014 at 05:50:28PM +0200, Andy Shevchenko wrote:
> There is nice kernel helper to escape a given strings by provided rules. Let's
> use it instead of custom approach.
Looks good, but it broke nfsd.
After staring at the patch for a while:
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> net/sunrpc/cache.c | 27 ++++++---------------------
> 1 file changed, 6 insertions(+), 21 deletions(-)
>
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 0663621..5cf60a4 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -20,6 +20,7 @@
> #include <linux/list.h>
> #include <linux/module.h>
> #include <linux/ctype.h>
> +#include <linux/string_helpers.h>
> #include <asm/uaccess.h>
> #include <linux/poll.h>
> #include <linux/seq_file.h>
> @@ -1067,32 +1068,16 @@ void qword_add(char **bpp, int *lp, char *str)
> {
> char *bp = *bpp;
> int len = *lp;
> - char c;
> + int ret;
>
> if (len < 0) return;
>
> - while ((c=*str++) && len)
> - switch(c) {
> - case ' ':
> - case '\t':
> - case '\n':
> - case '\\':
> - if (len >= 4) {
> - *bp++ = '\\';
> - *bp++ = '0' + ((c & 0300)>>6);
> - *bp++ = '0' + ((c & 0070)>>3);
> - *bp++ = '0' + ((c & 0007)>>0);
> - }
> - len -= 4;
> - break;
> - default:
> - *bp++ = c;
> - len--;
> - }
> - if (c || len <1) len = -1;
> + ret = string_escape_str(str, &bp, len, ESCAPE_OCTAL, "\\ \n\t");
> + if (ret < 0 || ret == len)
> + len = -1;
> else {
> *bp++ = ' ';
> - len--;
> + len -= ret - 1;
Looks like that should be ret + 1, not ret - 1. With that change,
things work.
Inclined to actually commit that as:
len -= ret;
*bp++ = ' ';
len--;
just to make the arithmetic really obvious.
--b.
> }
> *bpp = bp;
> *lp = len;
> --
> 2.1.3
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] sunrpc/cache: convert to use string_escape_str()
2014-12-08 22:38 ` J. Bruce Fields
@ 2014-12-09 9:24 ` Andy Shevchenko
2014-12-09 14:42 ` J. Bruce Fields
0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2014-12-09 9:24 UTC (permalink / raw)
To: J. Bruce Fields; +Cc: linux-nfs, David S. Miller
On Mon, 2014-12-08 at 17:38 -0500, J. Bruce Fields wrote:
> On Fri, Nov 28, 2014 at 05:50:28PM +0200, Andy Shevchenko wrote:
> > There is nice kernel helper to escape a given strings by provided rules. Let's
> > use it instead of custom approach.
>
> Looks good, but it broke nfsd.
>
> After staring at the patch for a while:
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > net/sunrpc/cache.c | 27 ++++++---------------------
> > 1 file changed, 6 insertions(+), 21 deletions(-)
> >
> > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> > index 0663621..5cf60a4 100644
> > --- a/net/sunrpc/cache.c
> > +++ b/net/sunrpc/cache.c
> > @@ -20,6 +20,7 @@
> > #include <linux/list.h>
> > #include <linux/module.h>
> > #include <linux/ctype.h>
> > +#include <linux/string_helpers.h>
> > #include <asm/uaccess.h>
> > #include <linux/poll.h>
> > #include <linux/seq_file.h>
> > @@ -1067,32 +1068,16 @@ void qword_add(char **bpp, int *lp, char *str)
> > {
> > char *bp = *bpp;
> > int len = *lp;
> > - char c;
> > + int ret;
> >
> > if (len < 0) return;
> >
> > - while ((c=*str++) && len)
> > - switch(c) {
> > - case ' ':
> > - case '\t':
> > - case '\n':
> > - case '\\':
> > - if (len >= 4) {
> > - *bp++ = '\\';
> > - *bp++ = '0' + ((c & 0300)>>6);
> > - *bp++ = '0' + ((c & 0070)>>3);
> > - *bp++ = '0' + ((c & 0007)>>0);
> > - }
> > - len -= 4;
> > - break;
> > - default:
> > - *bp++ = c;
> > - len--;
> > - }
> > - if (c || len <1) len = -1;
> > + ret = string_escape_str(str, &bp, len, ESCAPE_OCTAL, "\\ \n\t");
> > + if (ret < 0 || ret == len)
> > + len = -1;
> > else {
> > *bp++ = ' ';
> > - len--;
> > + len -= ret - 1;
>
> Looks like that should be ret + 1, not ret - 1. With that change,
> things work.
>
> Inclined to actually commit that as:
>
> len -= ret;
> *bp++ = ' ';
> len--;
>
> just to make the arithmetic really obvious.
>
> --b.
Good catch, thanks! It should decrement length indeed. In the form of -=
it must be + 1. Shall I resubmit patch? If so, can I include your tag
(Tested-by I suppose) ?
>
> > }
> > *bpp = bp;
> > *lp = len;
> > --
> > 2.1.3
> >
--
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v1] sunrpc/cache: convert to use string_escape_str()
2014-12-09 9:24 ` Andy Shevchenko
@ 2014-12-09 14:42 ` J. Bruce Fields
0 siblings, 0 replies; 4+ messages in thread
From: J. Bruce Fields @ 2014-12-09 14:42 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-nfs, David S. Miller
On Tue, Dec 09, 2014 at 11:24:09AM +0200, Andy Shevchenko wrote:
> On Mon, 2014-12-08 at 17:38 -0500, J. Bruce Fields wrote:
> > On Fri, Nov 28, 2014 at 05:50:28PM +0200, Andy Shevchenko wrote:
> > > There is nice kernel helper to escape a given strings by provided rules. Let's
> > > use it instead of custom approach.
> >
> > Looks good, but it broke nfsd.
> >
> > After staring at the patch for a while:
>
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > > ---
> > > net/sunrpc/cache.c | 27 ++++++---------------------
> > > 1 file changed, 6 insertions(+), 21 deletions(-)
> > >
> > > diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> > > index 0663621..5cf60a4 100644
> > > --- a/net/sunrpc/cache.c
> > > +++ b/net/sunrpc/cache.c
> > > @@ -20,6 +20,7 @@
> > > #include <linux/list.h>
> > > #include <linux/module.h>
> > > #include <linux/ctype.h>
> > > +#include <linux/string_helpers.h>
> > > #include <asm/uaccess.h>
> > > #include <linux/poll.h>
> > > #include <linux/seq_file.h>
> > > @@ -1067,32 +1068,16 @@ void qword_add(char **bpp, int *lp, char *str)
> > > {
> > > char *bp = *bpp;
> > > int len = *lp;
> > > - char c;
> > > + int ret;
> > >
> > > if (len < 0) return;
> > >
> > > - while ((c=*str++) && len)
> > > - switch(c) {
> > > - case ' ':
> > > - case '\t':
> > > - case '\n':
> > > - case '\\':
> > > - if (len >= 4) {
> > > - *bp++ = '\\';
> > > - *bp++ = '0' + ((c & 0300)>>6);
> > > - *bp++ = '0' + ((c & 0070)>>3);
> > > - *bp++ = '0' + ((c & 0007)>>0);
> > > - }
> > > - len -= 4;
> > > - break;
> > > - default:
> > > - *bp++ = c;
> > > - len--;
> > > - }
> > > - if (c || len <1) len = -1;
> > > + ret = string_escape_str(str, &bp, len, ESCAPE_OCTAL, "\\ \n\t");
> > > + if (ret < 0 || ret == len)
> > > + len = -1;
> > > else {
> > > *bp++ = ' ';
> > > - len--;
> > > + len -= ret - 1;
> >
> > Looks like that should be ret + 1, not ret - 1. With that change,
> > things work.
> >
> > Inclined to actually commit that as:
> >
> > len -= ret;
> > *bp++ = ' ';
> > len--;
> >
> > just to make the arithmetic really obvious.
> >
> > --b.
>
> Good catch, thanks! It should decrement length indeed. In the form of -=
> it must be + 1. Shall I resubmit patch? If so, can I include your tag
> (Tested-by I suppose) ?
I'll fix it up, thanks.
--b.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-09 14:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-28 15:50 [PATCH v1] sunrpc/cache: convert to use string_escape_str() Andy Shevchenko
2014-12-08 22:38 ` J. Bruce Fields
2014-12-09 9:24 ` Andy Shevchenko
2014-12-09 14:42 ` J. Bruce Fields
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox