On Mon, Jun 03, 2024 at 11:16:11PM -0700, Junio C Hamano wrote:
> * jc/t1517-more (2024-05-31) 1 commit
> (merged to 'next' on 2024-06-03 at 10b71e2a60)
> + t1517: more coverage for commands that work without repository
>
> "smoke tests" to ensure git commands that are designed to run
> outside repositories do work.
>
> Will merge to 'master'.
> source: <xmqqwmnajrrk.fsf@gitster.g>
This one seems to fail the CI leak jobs (I noticed it on next, but I
think even the tip of the feature branch fails).
One possible fix is this:
diff --git a/imap-send.c b/imap-send.c
index a5d1510180..26d5909e79 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1539,16 +1539,17 @@ int cmd_main(int argc, const char **argv)
/* read the messages */
if (strbuf_read(&all_msgs, 0, 0) < 0) {
error_errno(_("could not read from stdin"));
return 1;
}
if (all_msgs.len == 0) {
fprintf(stderr, "nothing to send\n");
+ strbuf_release(&all_msgs);
return 1;
}
total = count_messages(&all_msgs);
if (!total) {
fprintf(stderr, "no messages to send\n");
return 1;
}
But I wonder if strbuf_read() should handle the allocation itself when
it does a 0-byte read. We already do so for an error return (so the
"could not read from stdin" path above is actually OK).
Maybe a moot point, though. I think we _always_ leak the all_msgs
strbuf, so probably the whole function needs more unified cleanup. It
looks like Patrick's 11637fc740 (imap-send: fix leaking memory in
`imap_server_conf`, 2024-06-03) does that refactoring (but isn't yet in
'next'). Once ps/no-writable-strings is merged, we should be able to
just do:
diff --git a/imap-send.c b/imap-send.c
index da3e7ec17e..01404e5047 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1585,5 +1585,6 @@ int cmd_main(int argc, const char **argv)
free(server.user);
free(server.pass);
free(server.auth_method);
+ strbuf_release(&all_msgs);
return ret;
}
on top. Weirdly, with ps/no-writable-strings merged (but without the fix
above applied yet), SANITIZE=leak does not seem to find the leak
anymore! Even though I can confirm in a debugger or by printing the
strbuf's fields that it is still there. So that's...odd. But whatever is
going on with LSan, the fix above is the right thing.
-Peff
