[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