Kevin Daudt <m...@ikke.info> writes:
> +static int unescape_quoted_pair(struct mailinfo *mi, struct strbuf *line)
> +{
> + struct strbuf outbuf = STRBUF_INIT;
> + const char *in = line->buf;
> + int c, skip=0;
> + char escape_context=0;
Have SP around '=', i.e.
int c, skip = 0;
char escape_context = 0;
> + while ((c = *in++) != 0) {
> + if (!skip) {
> + switch (c) {
> + case '"':
> +...
> + break;
> + }
> + } else {
> + skip = 0;
> + }
> +
> + strbuf_addch(&outbuf, c);
> + }
It often is easier to read if smaller of the two are in the if part
and the larger in else part. Also your switch/case is indented one
level too deep. I.e.
while (...) {
if (skip) {
skip = 0;
} else {
switch (c) {
case '"':
do this;
...
}
}
strbuf_addch(...);
}
I found the variable name "skip" a bit hard to reason about. What
it does is to signal the next round of the processing that we have
seen a single-byte quote and it should keep the byte it will get, no
matter what its value is. It is "skipping" the conditional
processing, but I'd imagine most people would consider it is
"keeping the byte".
> @@ -461,6 +506,7 @@ static int check_header(struct mailinfo *mi,
> */
> strbuf_add(&sb, line->buf + len + 2, line->len - len -
> 2);
> decode_header(mi, &sb);
> + unescape_quoted_pair(mi, &sb);
> handle_header(&hdr_data[i], &sb);
> ret = 1;
> goto check_header_out;
I wonder why this call is only in here, not on other headers that
all call decode_header(). For that matter, I wonder if the call (or
the logic of the helper function itself) should go at the end of
decode_header(). After all, this is different kind of decoding; the
current one knows how to do b/q encoding but forgot about the more
traditional quoting done with backslash, and you are teaching the
code that the current decoding it does is insufficient and how to
handle the one that the original implementors forgot about.
Thanks.
