On Thu, May 07, 2026 at 11:03:43AM +0800, Kevin J. McCarthy wrote:
> On Wed, May 06, 2026 at 03:56:34PM -0400, Derek Martin wrote:
> > [...] concatenating paths correctly is substantially more
> > complicated to get right than you'd initially expect.
> 
> Please don't change the return type.

OK.

> Mutt's general policy is to not pass NULL strings around, instead it wraps
> parameters that might be NULL inside the NONULL() macro.

I was aware of this, vaguely, however when I checked it seemed like
this was mostly not happening.  Granted, I did not do any further
analysis to determine if it was possible for the arguments to be
NULL...

$ grep mutt_buffer_concat_path *.c
browser.c:    mutt_buffer_concat_path(full_path, d, de->d_name);
browser.c:              mutt_buffer_concat_path(tmp, mutt_b2s(working_dir), 
mutt_b2s(buf));
complete.c:    mutt_buffer_concat_path(imap_path, p, s+1);
hcache.c:  mutt_buffer_concat_path(hcpath, path, mutt_b2s(hcfile));
hook.c:      mutt_buffer_concat_path(path, NONULL(Maildir), mutt_b2s(buf));
init.c:    mutt_buffer_concat_path(buffer, NONULL(Homedir), MAILPATH);
init.c:    mutt_buffer_concat_path(buffer, MAILPATH, NONULL(Username));
muttlib.c:    mutt_buffer_concat_path(fname, path, mutt_b2s(tmp));
muttlib.c:void mutt_buffer_concat_path(BUFFER *d, const char *dir, const char 
*fname)
recvattach.c:      mutt_buffer_concat_path(buf, *directory, 
mutt_basename(body->filename));


> Similarly, if the caller passes a NULL "BUFFER *d" argument, let the
> program crash: that's just completely fubar.

Fair enough.

> So, deal with !*dir and !*fname, but don't check for dir and fname (or d)
> being NULL.

This will make the logic considerably simpler.

> Also please match the formatting style used in Mutt, i.e. Allman curly brace
> style, being free to omit the braces for single lines if you like.

Due to past experience, I don't do this unless the conditional statement fits
comfortably on the same line as the conditional.  It tends to lead to
bugs where someone expands what is done conditionally (or tries), but
they forget to add the braces, leading to a hard-to-see logic error.

> Previously, the code implicitly cleared the BUFFER (inside
> mutt_buffer_printf().  So you should call mutt_buffer_clear(d)
> before anything else.

Will do.


-- 
Derek D. Martin    http://www.pizzashack.org/   GPG Key ID: 0xDFBEAD02
-=-=-=-=-
This message is posted from an invalid address.  Replying to it will result in
undeliverable mail due to spam prevention.  Sorry for the inconvenience.

Reply via email to