Re: [PATCH 3/8] hex-escape: add function to decode escaped string in-place

[David Bremner <bremner@unb.ca>]

Jani Nikula <jani@nikula.org> writes:

> Add function hex_decode_inplace() to decode the input string onto
> itself.
>
> Signed-off-by: Jani Nikula <jani@nikula.org>
>
> ---
>
> This could be folded to "hex-escape: (en|de)code strings to/from
> restricted character set".

Probably. It's a bit hard to follow as is; somehow the code movement is
a bit noisy.

>      while (*p) {
> -
>  	if (*p == escape_char) {
> -
unrelated whitespace changes, naughty naughty.

> +hex_status_t
> +hex_decode_inplace (char *p)
> +{
> +    return hex_decode_internal (p, (unsigned char *) p);
this could probably use a comment to the effect that there _will_ be
enough space.

> +
> +    p = in;
> +    q = (unsigned char *) *out;

I understand trying to minimize changes, but do p and q serve any
purpose here?

> +
> +    return hex_decode_internal (p, q);
> +}


> +/*
> + * Decode 'in' onto itself.
> + */

This is just a bit terse for my taste.

d