From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934285AbbCDKvy (ORCPT ); Wed, 4 Mar 2015 05:51:54 -0500 Received: from mga11.intel.com ([192.55.52.93]:49397 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932756AbbCDKvx (ORCPT ); Wed, 4 Mar 2015 05:51:53 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,687,1418112000"; d="scan'208";a="660244471" Message-ID: <1425466309.14897.167.camel@linux.intel.com> Subject: Re: [PATCH v4 2/3] lib/string_helpers.c: Refactor string_escape_mem From: Andy Shevchenko To: Rasmus Villemoes Cc: Andrew Morton , linux-kernel@vger.kernel.org Date: Wed, 04 Mar 2015 12:51:49 +0200 In-Reply-To: <1425424836-17947-3-git-send-email-linux@rasmusvillemoes.dk> References: <1423525491-12613-1-git-send-email-linux@rasmusvillemoes.dk> <1425424836-17947-1-git-send-email-linux@rasmusvillemoes.dk> <1425424836-17947-3-git-send-email-linux@rasmusvillemoes.dk> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 2015-03-04 at 00:20 +0100, Rasmus Villemoes wrote: > When printf is given the format specifier %pE, it needs a way of > obtaining the total output size that would be generated if the buffer > was large enough, and string_escape_mem doesn't easily provide > that. This is a refactorization of string_escape_mem in preparation of > changing its external API to provide that information. > > The somewhat ugly early returns and subsequent seemingly redundant > conditionals are to make the following patch touch as little as > possible in string_helpers.c while still preserving the current > behaviour of never outputting partial escape sequences. That behaviour > must also change for %pE to work as one expects from every other > printf specifier. > > Signed-off-by: Rasmus Villemoes Thanks for an update! Acked-by: Andy Shevchenko > --- > lib/string_helpers.c | 208 ++++++++++++++++++++++++++------------------------- > 1 file changed, 105 insertions(+), 103 deletions(-) > > diff --git a/lib/string_helpers.c b/lib/string_helpers.c > index 8f8c4417f228..9c48ddad0f0d 100644 > --- a/lib/string_helpers.c > +++ b/lib/string_helpers.c > @@ -239,29 +239,21 @@ int string_unescape(char *src, char *dst, size_t size, unsigned int flags) > } > EXPORT_SYMBOL(string_unescape); > > -static int escape_passthrough(unsigned char c, char **dst, size_t *osz) > +static bool escape_passthrough(unsigned char c, char **dst, char *end) > { > char *out = *dst; > > - if (*osz < 1) > - return -ENOMEM; > - > - *out++ = c; > - > - *dst = out; > - *osz -= 1; > - > - return 1; > + if (out < end) > + *out = c; > + *dst = out + 1; > + return true; > } > > -static int escape_space(unsigned char c, char **dst, size_t *osz) > +static bool escape_space(unsigned char c, char **dst, char *end) > { > char *out = *dst; > unsigned char to; > > - if (*osz < 2) > - return -ENOMEM; > - > switch (c) { > case '\n': > to = 'n'; > @@ -279,26 +271,30 @@ static int escape_space(unsigned char c, char **dst, size_t *osz) > to = 'f'; > break; > default: > - return 0; > + return false; > } > > - *out++ = '\\'; > - *out++ = to; > + if (out + 2 > end) { > + *dst = out + 2; > + return true; > + } > > - *dst = out; > - *osz -= 2; > + if (out < end) > + *out = '\\'; > + ++out; > + if (out < end) > + *out = to; > + ++out; > > - return 1; > + *dst = out; > + return true; > } > > -static int escape_special(unsigned char c, char **dst, size_t *osz) > +static bool escape_special(unsigned char c, char **dst, char *end) > { > char *out = *dst; > unsigned char to; > > - if (*osz < 2) > - return -ENOMEM; > - > switch (c) { > case '\\': > to = '\\'; > @@ -310,71 +306,98 @@ static int escape_special(unsigned char c, char **dst, size_t *osz) > to = 'e'; > break; > default: > - return 0; > + return false; > } > > - *out++ = '\\'; > - *out++ = to; > + if (out + 2 > end) { > + *dst = out + 2; > + return true; > + } > > - *dst = out; > - *osz -= 2; > + if (out < end) > + *out = '\\'; > + ++out; > + if (out < end) > + *out = to; > + ++out; > > - return 1; > + *dst = out; > + return true; > } > > -static int escape_null(unsigned char c, char **dst, size_t *osz) > +static bool escape_null(unsigned char c, char **dst, char *end) > { > char *out = *dst; > > - if (*osz < 2) > - return -ENOMEM; > - > if (c) > - return 0; > + return false; > > - *out++ = '\\'; > - *out++ = '0'; > + if (out + 2 > end) { > + *dst = out + 2; > + return true; > + } > > - *dst = out; > - *osz -= 2; > + if (out < end) > + *out = '\\'; > + ++out; > + if (out < end) > + *out = '0'; > + ++out; > > - return 1; > + *dst = out; > + return true; > } > > -static int escape_octal(unsigned char c, char **dst, size_t *osz) > +static bool escape_octal(unsigned char c, char **dst, char *end) > { > char *out = *dst; > > - if (*osz < 4) > - return -ENOMEM; > + if (out + 4 > end) { > + *dst = out + 4; > + return true; > + } > > - *out++ = '\\'; > - *out++ = ((c >> 6) & 0x07) + '0'; > - *out++ = ((c >> 3) & 0x07) + '0'; > - *out++ = ((c >> 0) & 0x07) + '0'; > + if (out < end) > + *out = '\\'; > + ++out; > + if (out < end) > + *out = ((c >> 6) & 0x07) + '0'; > + ++out; > + if (out < end) > + *out = ((c >> 3) & 0x07) + '0'; > + ++out; > + if (out < end) > + *out = ((c >> 0) & 0x07) + '0'; > + ++out; > > *dst = out; > - *osz -= 4; > - > - return 1; > + return true; > } > > -static int escape_hex(unsigned char c, char **dst, size_t *osz) > +static bool escape_hex(unsigned char c, char **dst, char *end) > { > char *out = *dst; > > - if (*osz < 4) > - return -ENOMEM; > + if (out + 4 > end) { > + *dst = out + 4; > + return true; > + } > > - *out++ = '\\'; > - *out++ = 'x'; > - *out++ = hex_asc_hi(c); > - *out++ = hex_asc_lo(c); > + if (out < end) > + *out = '\\'; > + ++out; > + if (out < end) > + *out = 'x'; > + ++out; > + if (out < end) > + *out = hex_asc_hi(c); > + ++out; > + if (out < end) > + *out = hex_asc_lo(c); > + ++out; > > *dst = out; > - *osz -= 4; > - > - return 1; > + return true; > } > > /** > @@ -436,9 +459,10 @@ static int escape_hex(unsigned char c, char **dst, size_t *osz) > int string_escape_mem(const char *src, size_t isz, char **dst, size_t osz, > unsigned int flags, const char *esc) > { > - char *out = *dst, *p = out; > + char *p = *dst; > + char *end = p + osz; > bool is_dict = esc && *esc; > - int ret = 0; > + int ret; > > while (isz--) { > unsigned char c = *src++; > @@ -458,55 +482,33 @@ int string_escape_mem(const char *src, size_t isz, char **dst, size_t osz, > (is_dict && !strchr(esc, c))) { > /* do nothing */ > } else { > - if (flags & ESCAPE_SPACE) { > - ret = escape_space(c, &p, &osz); > - if (ret < 0) > - break; > - if (ret > 0) > - continue; > - } > - > - if (flags & ESCAPE_SPECIAL) { > - ret = escape_special(c, &p, &osz); > - if (ret < 0) > - break; > - if (ret > 0) > - continue; > - } > - > - if (flags & ESCAPE_NULL) { > - ret = escape_null(c, &p, &osz); > - if (ret < 0) > - break; > - if (ret > 0) > - continue; > - } > + if (flags & ESCAPE_SPACE && escape_space(c, &p, end)) > + continue; > + > + if (flags & ESCAPE_SPECIAL && escape_special(c, &p, end)) > + continue; > + > + if (flags & ESCAPE_NULL && escape_null(c, &p, end)) > + continue; > > /* ESCAPE_OCTAL and ESCAPE_HEX always go last */ > - if (flags & ESCAPE_OCTAL) { > - ret = escape_octal(c, &p, &osz); > - if (ret < 0) > - break; > + if (flags & ESCAPE_OCTAL && escape_octal(c, &p, end)) > continue; > - } > - if (flags & ESCAPE_HEX) { > - ret = escape_hex(c, &p, &osz); > - if (ret < 0) > - break; > + > + if (flags & ESCAPE_HEX && escape_hex(c, &p, end)) > continue; > - } > } > > - ret = escape_passthrough(c, &p, &osz); > - if (ret < 0) > - break; > + escape_passthrough(c, &p, end); > } > > - *dst = p; > - > - if (ret < 0) > - return ret; > + if (p > end) { > + *dst = end; > + return -ENOMEM; > + } > > - return p - out; > + ret = p - *dst; > + *dst = p; > + return ret; > } > EXPORT_SYMBOL(string_escape_mem); -- Andy Shevchenko Intel Finland Oy