linux-kernel - Re: [PATCH v2 2/3] lib/string_helpers.c: Refactor string_escape

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 29 Jan 2015 15:37:29 +0200
From:	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:	Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Mathias Krause <minipli@...glemail.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/3] lib/string_helpers.c: Refactor string_escape_mem

On Thu, 2015-01-29 at 14:10 +0100, Rasmus Villemoes wrote:
> On Thu, Jan 29 2015, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:


> >> -	*out++ = '\\';
> >> -	*out++ = to;
> >> +	if (out + 0 < end)
> >> +		out[0] = '\\';
> >> +	if (out + 1 < end)
> >> +		out[1] = to;
> >
> > Could we do this in the same way like for hex_string, i.e.
> >
> > if (out < end)
> >  *out = '\\';
> > ++out;
> >
> > …
> >
> > *dst = out;
> > return true;
> >
> > ?
> 
> We could, but I don't think either is more readable than the
> other. Hence I chose the one requiring 2n+1 lines instead of 3n+1
> lines. Had this been in vsprintf.c I would stick to the pattern you
> suggest.

I think the pattern with 3n lines is a bit more readable.

> >> +	ret = p - *dst;
> >>  	*dst = p;
> >> -
> >> -	if (ret < 0)
> >> -		return ret;
> >> -
> >> -	return p - out;
> >
> > …and do not change the logic right now. Just substitute if (ret < 0) by
> > above if (p > end).
> >
> 
> I'm not sure I follow. How does this change the logic? We return -ENOMEM
> if and only if the entire output didn't fit, while still updating *dst
> to point to one past the last output character. If the output did fit,
> we return the size of the output.

I meant to do the arithmetics like it was in the original code.

> One thing that only occured to me now is that we may now leave a partial
> escape sequence at the end of the buffer. I can't see how this can
> reasonably be avoided

I don't know which way is better, but you may check how it was done in
the original code. First it was checking for enough space, and only
after filling the destination buffer.

fs/proc/array.c, for example, originally might leave the back slash at
the end.

>  while still doing a meaningful refactorization
> preparing it for the next patch.
> 
> Rasmus


-- 
Andy Shevchenko <andriy.shevchenko@...el.com>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/