Hi Kevin,
On 2026-05-18T20:45:10+0800, Kevin J. McCarthy wrote:
> On Mon, May 18, 2026 at 01:03:29PM +0200, Alejandro Colomar via Mutt-dev
> wrote:
> > > + mailbox = safe_malloc(mutt_strlen(utf8_user) +
> > > mutt_strlen(utf8_domain) + 2);
> > > + sprintf(mailbox, "%s@%s", NONULL(utf8_user), NONULL(utf8_domain)); /*
> > > __SPRINTF_CHECKED__ */
> >
> > Should we use an sprintf(3) variant that allocates itself? That could
> > simplify this to:
> >
> > mailbox = aprintf("%s@%s", NONULL(utf8_user), NONULL(utf8_domain));
> > if (mailbox == NULL)
> > goto fail;
>
> Hi Alex,
>
> Actually sprintf() usage in mutt is pretty rare. The check_sec.sh script
> scans for it and requires an explicit comment for it to be allowed.
>
> The only reason I used it in this case is because it's used in the other two
> conversion functions in mutt_idna.c, and as I mentioned, I modelled this one
> off of those for consistency and ease of understanding.
>
> There is a safe_asprintf() equivalent in the mutt codebase in
> safe_asprintf.c, but surprisingly it's only used in four places in the code!
Hmmm, nice!
I think we could improve safe_asprintf() slightly, by returning the new
string, instead of passing a **pointer in a parameter. But those are
minor differences, and nothing urgent.
Compare usage:
char *scratch;
safe_asprintf(&scratch, "%s: %s", tag, value);
vs
char *scratch;
scratch = safe_aprintf("%s: %s", tag, value);
> I could change it here, but then I should probably change it in the
> other two functions to match. If you'd like I can make a separate patch
> to change it in all three places.
Yup, that would be good. :)
Cheers,
Alex
--
<https://www.alejandro-colomar.es>
signature.asc
Description: PGP signature
