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

"Kevin J. McCarthy" <[email protected]> Fri, 8 May 2026 12:57:09 +0800

Newsgroups	gmane.mail.mutt.devel
Message-ID	<af1tJYkfjb3QHrTt@qinghai>

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?

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

signature.asc (application/pgp-signature, 833 B)