Re: mutt_buffer_concat_path is broken [PATCH: 1/1] Version 2

Alejandro Colomar via Mutt-dev <[email protected]> Fri, 8 May 2026 14:40:56 +0200
Newsgroups	gmane.mail.mutt.devel
Message-ID	<af3ZdHFHl9USRmM-@devuan>
Hi Kevin,

On 2026-05-08T12:57:09+0800, Kevin J. McCarthy wrote:
> On Fri, May 08, 2026 at 06:08:37AM +0200, Rene Kita wrote:
> > On Thu, May 07, 2026 at 12:02:08PM -0400, Derek Martin wrote:
> > > Updated patch attached.  I think I addressed everything you mentioned.
> > > Tests still pass.
> 
> Yes, it did, thank you for your work, Derek.
> 
> > A little nitpicking below.
> 
> [...]
> 
> (In defense of Derek's commenting, I was the one who encouraged more
> verboseness and examples)
> 
> > If we don't care about adding empty strings we could just do (untested):
> > #v+
> > void mutt_buffer_concat_path(BUFFER *d, const char *dir, const char *fname)
> > {
> >  mutt_buffer_clear(d);
> >  mutt_buffer_addstr(d, dir);
> > 
> >  if (*dir && dir[strlen(dir)-1 != '/')
> >    mutt_buffer_addch(d, '/');
> >  while (*fname && *fname == '/') fname++;
> >  mutt_buffer_addstr(d, fname);
> > }
> > #v-
> 
> I like this version (subject to testing).
> 
> However, the last, simplest version that Alex posted also bears note. I'm
> adding more context lines (breaking the patch) to make the whole thing
> clearer:
> 
> diff --git i/muttlib.c w/muttlib.c
> index 59a48378..a9fe8779 100644
> --- i/muttlib.c
> +++ w/muttlib.c
> @@ -1394,6 +1394,8 @@ void mutt_buffer_concat_path(BUFFER *d, const char*dir, const char *fname)
>  {
>    const char *fmt = "%s/%s";
> 
>    if (!*fname || (*dir && dir[strlen(dir)-1] == '/'))
>      fmt = "%s%s";
> 
> +  fname += strspn(fname, "/");
> +
>    mutt_buffer_printf(d, fmt, dir, fname);
>  }
> 
> I think this is basically equivalent to what you posted, Rene.
> 
> The only tests that were failing in Derek's list where those in which fname
> started with '/'.  So it clearly fixes that bug.
> 
> Fortunately I don't have to choose.  I had forgotten that there is another
> version of the function in lib.c: mutt_concat_path().  It's not called, but
> it should be fixed too.  And Alex's one-line fix applies easily there.
> 
> So I'd like to apply Rene's version to the buffer version, swapping in the
> "fname += strspn(fname, "/");" in place of the while loop.  And Alex's
> version to the mutt_concat_path() (non-buffer) version.
> 
> Does that sound good to everyone?

Sounds good to me.  Seeing the actual patch with the two places would
help, but being the same thing I suggested, I'm 99% sure I'll like it.
Thanks!  :)


Have a lovely day!
Alex

> 
> -- 
> Kevin J. McCarthy
> GPG Fingerprint: 8975 A9B3 3AA3 7910 385C  5308 ADEF 7684 8031 6BDA



-- 
<https://www.alejandro-colomar.es>
signature.asc (application/pgp-signature, 833 B)