[PATCH 3/8] hex-escape: add function to decode escaped string in-place
Jani Nikula
jani at nikula.org
Thu Apr 5 05:00:50 PDT 2012
Thanks for the review, David. Agreed on all points.
J.
On Thu, 05 Apr 2012 08:56:24 -0300, David Bremner <bremner at unb.ca> wrote:
> Jani Nikula <jani at nikula.org> writes:
>
> > Add function hex_decode_inplace() to decode the input string onto
> > itself.
> >
> > Signed-off-by: Jani Nikula <jani at 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
More information about the notmuch
mailing list