Re: [PATCH] mailinfo.c::convert_to_utf8: reuse strlen info

From: Martin Ågren <martin.agren@gmail.com>
Date: Apr 18, 2020, 19:56 UTC
Message-ID: <CAN0heSppn6BBX4V1T1qgKc4XP+8i6qbcEqd1_3NqWQtZJLaJww@mail.gmail.com>
In-Reply-To: <20200418035449.9450-1-congdanhqx@gmail.com>

Hi Danh (I think that's how you want to be addressed?),

On Sat, 18 Apr 2020 at 06:00, Đoàn Trần Công Danh <congdanhqx@gmail.com> wrote:

Show 14 quoted lines

> @@ -447,19 +447,21 @@ static int convert_to_utf8(struct mailinfo *mi,
>                            struct strbuf *line, const char *charset)
>  {
>         char *out;
> +       size_t out_len;
>
>         if (!mi->metainfo_charset || !charset || !*charset)
>                 return 0;
>
>         if (same_encoding(mi->metainfo_charset, charset))
>                 return 0;
> -       out = reencode_string(line->buf, mi->metainfo_charset, charset);
> +       out = reencode_string_len(line->buf, line->len,
> +                                 mi->metainfo_charset, charset, &out_len);

This is equivalent as long as `line->len` is equal to `strlen(line->buf)`, which it will be (should be) because it's a strbuf. Ok.

Show 7 quoted lines

>         if (!out) {
>                 mi->input_error = -1;
>                 return error("cannot convert from %s to %s",
>                              charset, mi->metainfo_charset);
>         }
> -       strbuf_attach(line, out, strlen(out), strlen(out));
> +       strbuf_attach(line, out, out_len, out_len);

This conversion is ok as such. I wondered why we pass in the same value twice (before and after this patch). Turns out this usage is wrong (as per the documentation in strbuf.h) but safe (as per my understanding of the implementation in strbuf.c). I'll follow up with a series that fell out of that investigation.

>         return 0;
>  }

All in all, this conversion is correct and it doesn't leave the use of `strbuf_attach()` any less correct than it already was.

Martin