From patchwork Mon Sep 9 23:08:41 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797670 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AEBE118D64B for ; Mon, 9 Sep 2024 23:08:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923325; cv=none; b=A8CVzcqZhjNYZKb0JtQi3RYahjtOCl6CWkdM59g9udXpcmOdYAP2O02+bF6vfWYz8XyxAtQuivm+bTazxkRQ31L/gQbzYGTSGK2xXTveKReHxx4HhD6695KHTviHl5+VfObK2LpqelGROcErp6MlT4Wd83VaLhLqf+s2eTNyjPU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923325; c=relaxed/simple; bh=uih5asZmotTVf5whyLGP2t0FY0DqanZiKbJNHSk6H/A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=OfMgQlqEm1s1JdHv+I/92IO+lrLnlLjEvSF+OvtRtbIelOVULhbLAZ+0+liljLHsH2c7zCtTUuMRwMnedFOG1FVZSlnN0gQIgeSmQX4diTYDOEYcBfm5StNe9Yej4vhnnXqyMd/zUBzTe1N/cCgoWqeuomhXs0BwVug1jQ4HaY0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32189 invoked by uid 109); 9 Sep 2024 23:08:43 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:08:43 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24861 invoked by uid 111); 9 Sep 2024 23:08:42 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:08:42 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:08:41 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 1/9] t6300: drop newline from wrapped test title Message-ID: <20240909230841.GA921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> We don't usually include newlines in test titles, because you get funny TAP output like: ok 417 - show good signature with custom format ok 418 - show good signature with custom format with ssh ok 419 - signature atom with grade option and bad signature where a TAP parser would ignore the extra line anyway, giving the wrong title. This comes from 26c9c03f0a (ref-filter: add new "signature" atom, 2023-06-04), and I think it was probably just editor line wrapping. I checked for other cases with: git grep "test_expect_success [A-Z_,]* '[^']*$" git grep 'test_expect_success [A-Z_,]* "[^"]*$' but this was the only hit. Signed-off-by: Jeff King --- Just an annoyance I noticed while running t6300 over and over. t/t6300-for-each-ref.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 8d15713cc6..7c208e20a6 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -2003,8 +2003,7 @@ test_expect_success GPG 'show good signature with custom format' ' --format="$GRADE_FORMAT" >actual && test_cmp expect actual ' -test_expect_success GPGSSH 'show good signature with custom format - with ssh' ' +test_expect_success GPGSSH 'show good signature with custom format with ssh' ' test_config gpg.ssh.allowedSignersFile "${GPGSSH_ALLOWED_SIGNERS}" && FINGERPRINT=$(ssh-keygen -lf "${GPGSSH_KEY_PRIMARY}" | awk "{print \$2;}") && cat >expect.tmpl <<-\EOF && From patchwork Mon Sep 9 23:12:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797674 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C8F588F5A for ; Mon, 9 Sep 2024 23:12:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923551; cv=none; b=Wce//og1d0l4MZ1x0e5qc/JIHRIBuFuvoQ1/MmxfmseonfbR9Qz/whmG/kG6Vqnh8dDH2eVZBbgRKEvYNmxq9KgNDQTmyW9a5YKcG2B7GQOS6IJHmACYOmDsyusPqTJOyhMIQ0IB1d89QM7EeqHzfYmYQewW5mMJVoujWXHGkzM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923551; c=relaxed/simple; bh=6MAOrnPxgiYCabUVPkogXuNIk3fdOZxc+cCvMD2n1lg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WVxsmzFQfxiUwF4kWNogNVRduFJ0GDLLeWtsqcGxdcpG0F4TTk4FmwB2B374QsWf46eEMNdprZPpk0B+vXjNo2g31NkRL+K0nG8O/THzKErJfWYDqsGizGlbk+2Zm4jcYDqG0IkQtYZpEPqhBSTLTHJp6M9/TpHcN9uSZkKS6es= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32201 invoked by uid 109); 9 Sep 2024 23:12:29 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:12:29 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24904 invoked by uid 111); 9 Sep 2024 23:12:28 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:12:28 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:12:28 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 2/9] ref-filter: avoid extra copies of payload/signature Message-ID: <20240909231228.GB921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> When we know we're going to show the subject or body of a tag or commit, we call find_subpos(), which returns pointers and lengths for the three parts: subject, body, signature. Oddly, the function finds the signature twice: once by calling parse_signature() at the start, which copies the signature into a separate strbuf, and then again by calling parse_signed_buffer() after we've parsed past the subject. This is due to 482c119186 (gpg-interface: improve interface for parsing tags, 2021-02-11) and 88bce0e24c (ref-filter: hoist signature parsing, 2021-02-11). The idea is that in a multi-hash world, tag signatures may appear in the header, rather than at the end of the body, in which case we need to extract them into a separate buffer. But parse_signature() would never find such a buffer! It only looks for signature lines (like "-----BEGIN PGP") at the start of each line, without any header keyword. So this code will never find anything except the usual in-body signature. And the extra code has two downsides: 1. We spend time copying the payload and signature into strbufs. That might even be useful if we ended up with a NUL-terminated copy of the payload data, but we throw it away immediately. And the signature, since it comes at the end of the message, is already its own NUL-terminated buffer. The overhead isn't huge, but I measured a pretty consistent 1-2% speedup running "git for-each-ref --format='%(subject)'" with this patch on a clone of linux.git. 2. The output of find_subpos() is a set of three ptr/len combinations, but only two of them point into the original buffer. This makes the interface confusing: you can't do pointer comparisons between them, and you have to remember to free the signature buffer. Since there's only one caller, it's not too bad in practice, but it did bite me while working on the next patch (and simplifying it will pave the way for that). In the long run we might have to go back to something like this approach, if we do have multi-hash header signatures. But I would argue that the extra buffer should kick in only for a header signature, and be passed out of find_subpos() separately. Signed-off-by: Jeff King --- This should produce no behavior change. It does seem funny to me that we do this signature parsing for commits as well as tags, even through the former would have an in-header signature. So arguably we are wrongly cutting in-body signatures from commits, and not showing their signatures with %(contents:signature). (But note that %(signature) is its own thing and does handle commits correctly). So I don't know if we'd want to fix that or not, but I left it as-is in this series. And as I said above, if we did want to go that way, I think we'd still want to build it on top of this, so that find_subpos() returns the in-header and in-body signatures separately. ref-filter.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index b6c6c10127..0f5513ba7e 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1833,16 +1833,10 @@ static void find_subpos(const char *buf, size_t *nonsiglen, const char **sig, size_t *siglen) { - struct strbuf payload = STRBUF_INIT; - struct strbuf signature = STRBUF_INIT; const char *eol; const char *end = buf + strlen(buf); const char *sigstart; - /* parse signature first; we might not even have a subject line */ - parse_signature(buf, end - buf, &payload, &signature); - strbuf_release(&payload); - /* skip past header until we hit empty line */ while (*buf && *buf != '\n') { eol = strchrnul(buf, '\n'); @@ -1853,8 +1847,10 @@ static void find_subpos(const char *buf, /* skip any empty lines */ while (*buf == '\n') buf++; - *sig = strbuf_detach(&signature, siglen); + /* parse signature first; we might not even have a subject line */ sigstart = buf + parse_signed_buffer(buf, strlen(buf)); + *sig = sigstart; + *siglen = end - *sig; /* subject is first non-empty line */ *sub = buf; @@ -2021,7 +2017,6 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp v->s = xstrdup(subpos); } - free((void *)sigpos); } /* From patchwork Mon Sep 9 23:14:45 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797675 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 900CB18CBF0 for ; Mon, 9 Sep 2024 23:14:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923689; cv=none; b=XGh6tey/4m1J0g6WUhwRyDj9tXyUJtp8NHmqv6vu90fhS3t0Aov4sGOwSX1aPHTViNbcntUKF7dsjCXWmMwtGkxXrAtttTbIQBNRdXAP+gHvNyG2yb3/TzEJQWFU8jKgguhXmO7P5zNtq3k4UkCJosKdr3rOBU7F4v2J2bOmQyU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923689; c=relaxed/simple; bh=fXq9ROFAKuwTyESt0cQQyMmxnApP1zdpirZTc6rANLY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=N18BzrNuvRZe1B51UZA+seDUMll7jMsG8ln1PyhJ8Lx2FYlEO1N2OqphWgDY1DH51VLUGth8uarK16BkT7KyT0QlRLj4hsygXbaTiQbHHvGzq3fkt9SKPoLqBgXMv6gjOKZixBaDccdw910FJTqzc1t3pvIPdRXVZ+c3qte6TgQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32209 invoked by uid 109); 9 Sep 2024 23:14:47 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:14:47 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24913 invoked by uid 111); 9 Sep 2024 23:14:46 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:14:46 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:14:45 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 3/9] ref-filter: strip signature when parsing tag trailers Message-ID: <20240909231445.GC921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> To expand the "%(trailers)" placeholder, we have to feed the commit or tag body to the trailer API. But that API doesn't know anything about signatures, and will be confused by a signed tag like this: the subject the body Some-trailer: foo -----BEGIN PGP SIGNATURE----- ...etc... because it will start looking for trailers after the signature, and get stopped walking backwards by the very non-trailer signature lines. So it thinks there are no trailers. This problem has existed since %(trailers) was added to the ref-filter code, but back then trailers on tags weren't something we really considered (commits don't have the same problem because their signatures are embedded in the header). But since 066cef7707 (builtin/tag: add --trailer option, 2024-05-05), we'd generate an object like the above for "git tag -s --trailer 'Some-trailer: foo' my-tag". The implementation here is pretty simple: we just make a NUL-terminated copy of the non-signature part of the tag (which we've already parsed) and pass it to the trailer API. There are some alternatives I rejected, at least for now: - the trailer code already understands skipping past some cruft at the end of a commit, such as patch dividers. see find_end_of_log_message(). We could teach it to do the same for signatures. But since this is the only context where we'd want that feature, and since we've already parsed the object into subject/body/signature here, it seemed easier to just pass in the truncated message. - it would be nice if we could just pass in a pointer/len pair to the trailer API (rather than a NUL-terminated string) to avoid the extra copy. I think this is possible, since as noted above, the trailer code already has to deal with ignoring some cruft at the end of the input. But after an initial attempt at this, it got pretty messy, as we have to touch a lot of intermediate functions that are also called in other contexts. So I went for the simple and stupid thing, at least for now. I don't think the extra copy overhead will be all that bad. The previous patch noted that an extra copy seemed to cause about 1-2% slowdown for something simple like "%(subject)". But here we are only triggering it for "%(trailers)" (and only when there is a signature), and the trailer code is a bit allocation-heavy already. I couldn't measure any difference formatting "%(trailers)" on linux.git before and after (even though there are not even any trailers to find). Reported-by: Brooke Kuhlmann Signed-off-by: Jeff King --- ref-filter.c | 10 +++++++++- t/t6300-for-each-ref.sh | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index 0f5513ba7e..e39f505a81 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2008,9 +2008,17 @@ static void grab_sub_body_contents(struct atom_value *val, int deref, struct exp v->s = strbuf_detach(&s, NULL); } else if (atom->u.contents.option == C_TRAILERS) { struct strbuf s = STRBUF_INIT; + const char *msg; + char *to_free = NULL; + + if (siglen) + msg = to_free = xmemdupz(subpos, sigpos - subpos); + else + msg = subpos; /* Format the trailer info according to the trailer_opts given */ - format_trailers_from_commit(&atom->u.contents.trailer_opts, subpos, &s); + format_trailers_from_commit(&atom->u.contents.trailer_opts, msg, &s); + free(to_free); v->s = strbuf_detach(&s, NULL); } else if (atom->u.contents.option == C_BARE) diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index 7c208e20a6..b830b542c4 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -1835,6 +1835,24 @@ sig_crlf="$(printf "%s" "$sig" | append_cr; echo dummy)" sig_crlf=${sig_crlf%dummy} test_atom refs/tags/fake-sig-crlf contents:signature "$sig_crlf" +test_expect_success 'set up tag with signature and trailers' ' + git tag -F - fake-sig-trailer <<-\EOF + this is the subject + + this is the body + + My-Trailer: foo + -----BEGIN PGP SIGNATURE----- + + not a real signature, but we just care about the + subject/body/trailer parsing. + -----END PGP SIGNATURE----- + EOF +' + +# use "separator=" here to suppress the terminating newline +test_atom refs/tags/fake-sig-trailer trailers:separator= 'My-Trailer: foo' + test_expect_success 'git for-each-ref --stdin: empty' ' >in && git for-each-ref --format="%(refname)" --stdin actual && From patchwork Mon Sep 9 23:14:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797676 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 74AC418DF73 for ; Mon, 9 Sep 2024 23:15:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923703; cv=none; b=ahfuOp3zh1pVzCSw3Y7esGwnCXf5GBWDei0tWNI28i5HiKg+9dA7a4thBYIKfVEeR626AJrocSJXU+AmybfkCGiYrmt69CH0s9/Apil/H8wDaSsTUONmyP9W2uh2Fjme3kxyooNlUkq3cWEEpj2ypnAKCS3MzPMuZ4zqtVbGeBA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923703; c=relaxed/simple; bh=O5S/vXqj3PvJpQMcNKRuGxvupG1Wkh1AsfRavo9x2os=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=HUHTRJ624IF53a+Z/gEZqksYFl5+OY6EtX+o+/JJIgjXyMuyi0ES16h5XJbuNzMOtDgzNnzwCtkHeKlK9kFEBPBZvuFtm5VVxhy2bjrZbHe7EiRTCq4yjI25vPLH7eEJY23LGHzR0d1Tl8rwbNV4oMAGAOvXMx0EGSEl1VWtvQE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32221 invoked by uid 109); 9 Sep 2024 23:15:00 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:15:00 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24925 invoked by uid 111); 9 Sep 2024 23:15:00 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:15:00 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:14:59 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 4/9] ref-filter: drop useless cast in trailers_atom_parser() Message-ID: <20240909231459.GD921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> There's no need to cast invalid_arg before freeing it. It is already a non-const pointer. Signed-off-by: Jeff King --- ref-filter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ref-filter.c b/ref-filter.c index e39f505a81..4d0df546da 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -578,7 +578,7 @@ static int trailers_atom_parser(struct ref_format *format UNUSED, strbuf_addf(err, _("expected %%(trailers:key=)")); else strbuf_addf(err, _("unknown %%(trailers) argument: %s"), invalid_arg); - free((char *)invalid_arg); + free(invalid_arg); return -1; } } From patchwork Mon Sep 9 23:16:53 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797683 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A4038210EC for ; Mon, 9 Sep 2024 23:16:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923817; cv=none; b=iQdRTcMOZO5Ng5G9s4n+CGqnYGCayDpVKdyIkBw2maTzIi0KoxJnNTUqxA596/rK4rMfuvx7bppU3OqkM1vVUWGF/r6MfxL8C+toFdyQfFwTNI000/NNYa9Gto+RsfM5fcx5fBcmRmc/JlQsYfrLt6UdIYDgw1XDyEu9vwz3Cu8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923817; c=relaxed/simple; bh=8Ht51ueZy27CHoEBJztzQvbnsMmiMWFJZ32NYilwoNA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=IHGJj5mTzxksKOm4RWoy2kxa0dpOVZRRq2rP/cRbcxmN75hT4VElLADq3YcG1EDz96RjCbKM5Q/cJF+bGlHwIEeFxvLCrVlStceGfR7aGmdxAsP3tK0WmOyXALLtEGxk9j2ntkcotxdxEaCqjtnciYZarmTlXuOcXfLyDKz4Pm8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32229 invoked by uid 109); 9 Sep 2024 23:16:55 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:16:55 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24955 invoked by uid 111); 9 Sep 2024 23:16:54 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:16:54 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:16:53 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 5/9] ref-filter: store ref_trailer_buf data per-atom Message-ID: <20240909231653.GE921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> The trailer API takes options via a trailer_opts struct. Some of those options point to data structures which require extra storage. Those structures aren't actually embedded in the options struct, but rather we pass pointers, and the caller is responsible for managing them. This is a little convoluted, but makes sense since some of them are not even concrete (e.g., you can pass a filter function and a void data pointer, but the trailer code doesn't even know what's in the pointer). When for-each-ref, etc, parse the %(trailers) placeholder, they stuff the extra data into a ref_trailer_buf struct. But we only hold a single static global instance of this struct. So if a format string has multiple %(trailer) placeholders, they'll stomp on each other: the "key" list will end up with entries for all of them, and the separator buffers will use the values from whichever was parsed last. Instead, we should have a ref_trailer_buf for each instance of the placeholder, and store it alongside the trailer_opts in the used_atom structure. And that's what this patch does. Note that we also have to add code to clean them up in ref_array_clear(). The original code did not bother cleaning them up, but it wasn't technically a "leak" since they were still reachable from the static global instance. Reported-by: Brooke Kuhlmann Signed-off-by: Jeff King --- ref-filter.c | 36 ++++++++++++++++++++++++++++++------ t/t6300-for-each-ref.sh | 19 +++++++++++++++++++ 2 files changed, 49 insertions(+), 6 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 4d0df546da..ebddc041c7 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -75,11 +75,11 @@ struct refname_atom { int lstrip, rstrip; }; -static struct ref_trailer_buf { +struct ref_trailer_buf { struct string_list filter_list; struct strbuf sepbuf; struct strbuf kvsepbuf; -} ref_trailer_buf = {STRING_LIST_INIT_NODUP, STRBUF_INIT, STRBUF_INIT}; +}; static struct expand_data { struct object_id oid; @@ -201,6 +201,7 @@ static struct used_atom { enum { C_BARE, C_BODY, C_BODY_DEP, C_LENGTH, C_LINES, C_SIG, C_SUB, C_SUB_SANITIZE, C_TRAILERS } option; struct process_trailer_options trailer_opts; + struct ref_trailer_buf *trailer_buf; unsigned int nlines; } contents; struct { @@ -568,12 +569,24 @@ static int trailers_atom_parser(struct ref_format *format UNUSED, if (arg) { const char *argbuf = xstrfmt("%s)", arg); char *invalid_arg = NULL; + struct ref_trailer_buf *tb; + + /* + * Do not inline these directly into the used_atom struct! + * When we parse them in format_set_trailers_options(), + * we will make pointer references directly to them, + * which will not survive a realloc() of the used_atom list. + * They must be allocated in a separate, stable struct. + */ + atom->u.contents.trailer_buf = tb = xmalloc(sizeof(*tb)); + string_list_init_nodup(&tb->filter_list); + strbuf_init(&tb->sepbuf, 0); + strbuf_init(&tb->kvsepbuf, 0); if (format_set_trailers_options(&atom->u.contents.trailer_opts, - &ref_trailer_buf.filter_list, - &ref_trailer_buf.sepbuf, - &ref_trailer_buf.kvsepbuf, - &argbuf, &invalid_arg)) { + &tb->filter_list, + &tb->sepbuf, &tb->kvsepbuf, + &argbuf, &invalid_arg)) { if (!invalid_arg) strbuf_addf(err, _("expected %%(trailers:key=)")); else @@ -2988,6 +3001,17 @@ void ref_array_clear(struct ref_array *array) struct used_atom *atom = &used_atom[i]; if (atom->atom_type == ATOM_HEAD) free(atom->u.head); + else if (atom->atom_type == ATOM_TRAILERS || + (atom->atom_type == ATOM_CONTENTS && + atom->u.contents.option == C_TRAILERS)) { + struct ref_trailer_buf *tb = atom->u.contents.trailer_buf; + if (tb) { + string_list_clear(&tb->filter_list, 0); + strbuf_release(&tb->sepbuf); + strbuf_release(&tb->kvsepbuf); + free(tb); + } + } free((char *)atom->name); } FREE_AND_NULL(used_atom); diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index b830b542c4..e8db612f95 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -1560,6 +1560,25 @@ test_trailer_option '%(trailers:separator,key_value_separator) changes both sepa Reviewed-by,A U Thor ,Signed-off-by,A U Thor EOF +test_expect_success 'multiple %(trailers) use their own options' ' + git tag -F - tag-with-trailers <<-\EOF && + body + + one: foo + one: bar + two: baz + two: qux + EOF + t1="%(trailers:key=one,key_value_separator=W,separator=X)" && + t2="%(trailers:key=two,key_value_separator=Y,separator=Z)" && + git for-each-ref --format="$t1%0a$t2" refs/tags/tag-with-trailers >actual && + cat >expect <<-\EOF && + oneWfooXoneWbar + twoYbazZtwoYqux + EOF + test_cmp expect actual +' + test_failing_trailer_option () { title=$1 option=$2 cat >expect From patchwork Mon Sep 9 23:18:28 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797684 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 14D03210EC for ; Mon, 9 Sep 2024 23:18:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923911; cv=none; b=HO8SqpglVxqUD2js9coQqFp0qRRubmjG243EIrSpMVudUOvC9ZHoHILfRMJK25Kh9ZSwqc8VTI1KjoImhbQM5ijACKSqF3f/EixzrMuKFoIGRpgDk262JgwbKYs54pXUCk1Sbx1/Da9gZvWJRCEHPUEw02fA8hEWRwxhNBCqN9g= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923911; c=relaxed/simple; bh=5W84GesuWOqpWj86ZaTxUz+YMEE2OElo2bKmBakVjFY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BnjswJ1EPpoYcq6bwwgMgePk0vQwXLW6z7QDlH+Ss2zu4BERDNWviIBfYfTNI9dGaKXT0x3XNJKODz5IzI2Q1vM8D3igpO8Q2Pj15yqkTQLGu0tvOQqJPQo6YRgVDtagWfew8euAjA/7iSzjz7ZvvBL3XVuF6k09+4AXUhhe3bU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32243 invoked by uid 109); 9 Sep 2024 23:18:29 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:18:29 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24969 invoked by uid 111); 9 Sep 2024 23:18:28 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:18:28 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:18:28 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 6/9] ref-filter: fix leak of %(trailers) "argbuf" Message-ID: <20240909231828.GF921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> When we parse a placeholder like "%(trailers:key=foo)", our atom parsing function is passed just the argument string "key=foo". We duplicate this into its own string, but never free it, causing a leak. We do the duplication for two reasons: 1. There's a mismatch with the pretty.c trailer-formatting code that we rely on. It expects to see a closing paren, like "key=foo)". So we duplicate the argument string with that extra character to pass along. This is probably something we could fix in the long run, but it's somewhat non-trivial if we want to avoid regressing error cases for things like "git log --format='%(trailer:oops'". So let's accept it as a necessity for now. 2. The argument parser expects to store the list of "key" entries ("foo" in this case) in a string-list. It also stores the length of the string in the string-list "util" field. The original caller in pretty.c uses this with a "nodup" string list to avoid making extra copies, which creates a subtle dependency on the lifetime of the original format string. We do the same here, which creates that same dependency. So we can't simply free it as soon as the parsing is done. There are two possible solutions here. The first is to hold on to the duplicated "argbuf" string in the used_atom struct, so that it lives as long as the string_list which references it. But I think a less-subtle solution, and what this patch does, is to switch to a duplicating string_list. That makes it self-contained, and lets us free argbuf immediately. It may involve a few extra allocations, but this parsing is something that happens once per program, not once per output ref. This clears up one case that LSan finds in t6300, but there are more. Signed-off-by: Jeff King --- ref-filter.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index ebddc041c7..54c5079dde 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -567,7 +567,8 @@ static int trailers_atom_parser(struct ref_format *format UNUSED, atom->u.contents.trailer_opts.no_divider = 1; if (arg) { - const char *argbuf = xstrfmt("%s)", arg); + char *argbuf = xstrfmt("%s)", arg); + const char *arg = argbuf; char *invalid_arg = NULL; struct ref_trailer_buf *tb; @@ -579,21 +580,23 @@ static int trailers_atom_parser(struct ref_format *format UNUSED, * They must be allocated in a separate, stable struct. */ atom->u.contents.trailer_buf = tb = xmalloc(sizeof(*tb)); - string_list_init_nodup(&tb->filter_list); + string_list_init_dup(&tb->filter_list); strbuf_init(&tb->sepbuf, 0); strbuf_init(&tb->kvsepbuf, 0); if (format_set_trailers_options(&atom->u.contents.trailer_opts, &tb->filter_list, &tb->sepbuf, &tb->kvsepbuf, - &argbuf, &invalid_arg)) { + &arg, &invalid_arg)) { if (!invalid_arg) strbuf_addf(err, _("expected %%(trailers:key=)")); else strbuf_addf(err, _("unknown %%(trailers) argument: %s"), invalid_arg); free(invalid_arg); + free(argbuf); return -1; } + free(argbuf); } atom->u.contents.option = C_TRAILERS; return 0; From patchwork Mon Sep 9 23:19:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797685 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DEA5B210EC for ; Mon, 9 Sep 2024 23:19:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923945; cv=none; b=pWb/quPYk/UeM/GEko5gvr4MuStDwAENcM19fMRCDBbbTE6ckXelfHbXBwFHUGor+b+ph0mYCtwFdWhwFWHr5Pk0X+EQXv7YHO42Y6jUTBoXEWKR5LYaYg5GqN1x7cTu7UGMzUMeqFCeJIysk7VMO6ikW917wk5/x72pbIBpGWo= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923945; c=relaxed/simple; bh=3IKYQ4rDaj1foKll4xoRe0xu68VP9pZIhvafL9CNWFU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hCCiDBpTQbudmGt+nDvfcyNvPDMWXbRgMHl5ecKt0fFim9Hi/zUMx+LLjbLryzQTGIB8BMG0zm84snIy9hYW6JS2rqzSSeylyfqxyMjU5C+DsANHGYk5RkdAyFQjh6Gl/l1hznxSZsPbUXZSsxP3TaOOuJVJgU6JbdE/9F5sdzc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32252 invoked by uid 109); 9 Sep 2024 23:19:03 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:19:03 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24975 invoked by uid 111); 9 Sep 2024 23:19:02 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:19:02 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:19:02 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 7/9] ref-filter: fix leak with %(describe) arguments Message-ID: <20240909231902.GG921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> When we parse a %(describe) placeholder, we stuff its arguments into a strvec, which is then detached into the used_atom struct. But later, when ref_array_clear() frees the atom, we never free the memory. To solve this, we just need to add the appropriate free() calls. But it's a little awkward, since we have to free each element of the array, in addition to the array itself. Instead, let's store the actual strvec, which lets us do a simple strvec_clear(). This clears up one case that LSan finds in t6300, but there are more. Signed-off-by: Jeff King --- ref-filter.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 54c5079dde..370cc5b44a 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -233,7 +233,7 @@ static struct used_atom { enum { S_BARE, S_GRADE, S_SIGNER, S_KEY, S_FINGERPRINT, S_PRI_KEY_FP, S_TRUST_LEVEL } option; } signature; - const char **describe_args; + struct strvec describe_args; struct refname_atom refname; char *head; } u; @@ -693,7 +693,7 @@ static int describe_atom_parser(struct ref_format *format UNUSED, struct used_atom *atom, const char *arg, struct strbuf *err) { - struct strvec args = STRVEC_INIT; + strvec_init(&atom->u.describe_args); for (;;) { int found = 0; @@ -702,13 +702,12 @@ static int describe_atom_parser(struct ref_format *format UNUSED, if (!arg || !*arg) break; - found = describe_atom_option_parser(&args, &arg, err); + found = describe_atom_option_parser(&atom->u.describe_args, &arg, err); if (found < 0) return found; if (!found) return err_bad_arg(err, "describe", bad_arg); } - atom->u.describe_args = strvec_detach(&args); return 0; } @@ -1941,7 +1940,7 @@ static void grab_describe_values(struct atom_value *val, int deref, cmd.git_cmd = 1; strvec_push(&cmd.args, "describe"); - strvec_pushv(&cmd.args, atom->u.describe_args); + strvec_pushv(&cmd.args, atom->u.describe_args.v); strvec_push(&cmd.args, oid_to_hex(&commit->object.oid)); if (pipe_command(&cmd, NULL, 0, &out, 0, &err, 0) < 0) { error(_("failed to run 'describe'")); @@ -3004,6 +3003,8 @@ void ref_array_clear(struct ref_array *array) struct used_atom *atom = &used_atom[i]; if (atom->atom_type == ATOM_HEAD) free(atom->u.head); + else if (atom->atom_type == ATOM_DESCRIBE) + strvec_clear(&atom->u.describe_args); else if (atom->atom_type == ATOM_TRAILERS || (atom->atom_type == ATOM_CONTENTS && atom->u.contents.option == C_TRAILERS)) { From patchwork Mon Sep 9 23:19:51 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797686 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4DB07130495 for ; Mon, 9 Sep 2024 23:19:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923994; cv=none; b=LAmk5p6r2+CX1VfmKn7uckBryxYIAgqRqZvn4c/N1YeMw7LMjnGyQfmvT2N76e7yYqBUSgpJRz78s9QDpEryMqlUroS29yRLHmQJFIJgLoWJuBwUErHfh4Ec8tqeK1T+EoDkI++nflWUPGWcehVQ5DllwEM5xic7Pfu1STn/FQE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725923994; c=relaxed/simple; bh=gontoa4bmp+ewbBGtc/f8Xwy0VRZviWOD3yk4GeDJcc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=tbhtxKVvr1K3MUS8BxrRfPDmq4eKz6wnxOWhA3GSsORsYgCx6fuEwV/anjTiLz7HghWsI3Nsi6a8gkNh8fusXaSK4Vu0bQl6erniStA6qD3fLPf6vDo0ijZuhzxtpe8qxfyfW/g4/KpF13rcJA8gb4sdJAPNxFoibqwe1NO3Jrc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32256 invoked by uid 109); 9 Sep 2024 23:19:52 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:19:52 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 24983 invoked by uid 111); 9 Sep 2024 23:19:52 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:19:52 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:19:51 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 8/9] ref-filter: fix leak when formatting %(push:remoteref) Message-ID: <20240909231951.GH921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> When we expand the %(upstream) or %(push) placeholders, we rely on remote.c's remote_ref_for_branch() to fill in the ":refname" argument. But that function has confusing memory ownership semantics: it may or may not return an allocated string, depending on whether we are in "upstream" mode or "push" mode. The caller in ref-filter.c always duplicates the result, meaning that we leak the original in the case of %(push:refname). To solve this, let's make the return value from remote_ref_for_branch() consistent, by always returning an allocated pointer. Note that the switch to returning a non-const pointer has a ripple effect inside the function, too. We were storing the "dst" result as a const pointer, too, even though it is always allocated! It is the return value from apply_refspecs(), which is always a non-const allocated string. And then on the caller side in ref-filter.c (and this is the only caller at all), we just need to avoid the extra duplication when the return value is non-NULL. This clears up one case that LSan finds in t6300, but there are more. Signed-off-by: Jeff King --- ref-filter.c | 2 +- remote.c | 8 ++++---- remote.h | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index 370cc5b44a..0f51095bbd 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -2237,7 +2237,7 @@ static void fill_remote_ref_details(struct used_atom *atom, const char *refname, const char *merge; merge = remote_ref_for_branch(branch, atom->u.remote_ref.push); - *s = xstrdup(merge ? merge : ""); + *s = merge ? merge : xstrdup(""); } else BUG("unhandled RR_* enum"); } diff --git a/remote.c b/remote.c index 8f3dee1318..539e5ceae3 100644 --- a/remote.c +++ b/remote.c @@ -632,19 +632,19 @@ const char *pushremote_for_branch(struct branch *branch, int *explicit) static struct remote *remotes_remote_get(struct remote_state *remote_state, const char *name); -const char *remote_ref_for_branch(struct branch *branch, int for_push) +char *remote_ref_for_branch(struct branch *branch, int for_push) { read_config(the_repository, 0); die_on_missing_branch(the_repository, branch); if (branch) { if (!for_push) { if (branch->merge_nr) { - return branch->merge_name[0]; + return xstrdup(branch->merge_name[0]); } } else { - const char *dst, - *remote_name = remotes_pushremote_for_branch( + char *dst; + const char *remote_name = remotes_pushremote_for_branch( the_repository->remote_state, branch, NULL); struct remote *remote = remotes_remote_get( diff --git a/remote.h b/remote.h index b901b56746..a58713f20a 100644 --- a/remote.h +++ b/remote.h @@ -329,7 +329,7 @@ struct branch { struct branch *branch_get(const char *name); const char *remote_for_branch(struct branch *branch, int *explicit); const char *pushremote_for_branch(struct branch *branch, int *explicit); -const char *remote_ref_for_branch(struct branch *branch, int for_push); +char *remote_ref_for_branch(struct branch *branch, int for_push); /* returns true if the given branch has merge configuration given. */ int branch_has_merge_config(struct branch *branch); From patchwork Mon Sep 9 23:21:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13797693 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 26578130495 for ; Mon, 9 Sep 2024 23:21:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725924081; cv=none; b=aUZlRo86ylZD07hKHBepAZJnIy6NQ1hSMOExTnPkp01tUKDegPsCPlCcEU4zscbqXVUOUYvmGFQ1fLG1eFAUBrCrViOJfPmpuz2fHfHf3k5ttIf9aINZD+UNlMyyqHBISuoZxwCFo7E3pp/VfK49LALHUAJ3uCQoESUPNW+NN34= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725924081; c=relaxed/simple; bh=j76Fo+wy1YH0Zw0XaEHaUKS5MJ0BrQZ9uR0qKsjEVAg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=oHonlg13JR+CabSSw/o8klxP4Hnan8WLKiIq5bvDElQFStKY83RMHgIFe4VaZI9H+lEWfDnzlp/NUZQU0UrFRbzyKhPqIEqIAv8he3NJzJdUALjNZU4EenXb0iJvtIY1O4pp+vVSsBAkjq5SwgMtdlZsBxi6257eN7LOm7Cinm4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Received: (qmail 32267 invoked by uid 109); 9 Sep 2024 23:21:19 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Mon, 09 Sep 2024 23:21:19 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 25024 invoked by uid 111); 9 Sep 2024 23:21:18 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Mon, 09 Sep 2024 19:21:18 -0400 Authentication-Results: peff.net; auth=none Date: Mon, 9 Sep 2024 19:21:18 -0400 From: Jeff King To: git@vger.kernel.org Cc: Brooke Kuhlmann Subject: [PATCH 9/9] ref-filter: add ref_format_clear() function Message-ID: <20240909232118.GI921834@coredump.intra.peff.net> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> After using the ref-filter API, callers should use ref_filter_clear() to free any used memory. However, there's not a matching function to clear the ref_format struct. Traditionally this did not need to be cleaned up, as it was just a way for the caller to store and pass format options as a single unit. Even though the parsing step of some placeholders may allocate data, that's usually inside their "used_atom" structs, which are part of the ref_filter itself. But a few placeholders keep data outside of there. The %(ahead-behind) and %(is-base) parsers both keep a master list of bases, because they perform a single filtering pass outside of the use of any particular atom. And since the format parser does not have access to the ref_filter struct, they store their cross-atom data in the ref_format struct itself. And thus when they are finished, the ref_format also needs to be cleaned up. So let's add a function to do so, and call it from all of the users of the ref-filter API. The %(is-base) case is found by running LSan on t6300. After this patch, the script can now be marked leak-free. Signed-off-by: Jeff King --- builtin/branch.c | 1 + builtin/for-each-ref.c | 1 + builtin/tag.c | 1 + builtin/verify-tag.c | 1 + ref-filter.c | 13 +++++++++++++ ref-filter.h | 3 +++ t/t6300-for-each-ref.sh | 1 + 7 files changed, 21 insertions(+) diff --git a/builtin/branch.c b/builtin/branch.c index 3f870741bf..c98601c6fe 100644 --- a/builtin/branch.c +++ b/builtin/branch.c @@ -878,6 +878,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix) string_list_clear(&output, 0); ref_sorting_release(sorting); ref_filter_clear(&filter); + ref_format_clear(&format); return 0; } else if (edit_description) { const char *branch_name; diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c index 5517a4a1c0..c72fa05bcb 100644 --- a/builtin/for-each-ref.c +++ b/builtin/for-each-ref.c @@ -104,6 +104,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix) filter_and_format_refs(&filter, flags, sorting, &format); ref_filter_clear(&filter); + ref_format_clear(&format); ref_sorting_release(sorting); strvec_clear(&vec); return 0; diff --git a/builtin/tag.c b/builtin/tag.c index a1fb218512..607e48e311 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -702,6 +702,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) cleanup: ref_sorting_release(sorting); ref_filter_clear(&filter); + ref_format_clear(&format); strbuf_release(&buf); strbuf_release(&ref); strbuf_release(&reflog_msg); diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c index c731e2f87b..77becf7e75 100644 --- a/builtin/verify-tag.c +++ b/builtin/verify-tag.c @@ -65,5 +65,6 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix) if (format.format) pretty_print_ref(name, &oid, &format); } + ref_format_clear(&format); return had_error; } diff --git a/ref-filter.c b/ref-filter.c index 0f51095bbd..ce1bcfad85 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -3621,3 +3621,16 @@ void ref_filter_clear(struct ref_filter *filter) free_commit_list(filter->unreachable_from); ref_filter_init(filter); } + +void ref_format_init(struct ref_format *format) +{ + struct ref_format blank = REF_FORMAT_INIT; + memcpy(format, &blank, sizeof(blank)); +} + +void ref_format_clear(struct ref_format *format) +{ + string_list_clear(&format->bases, 0); + string_list_clear(&format->is_base_tips, 0); + ref_format_init(format); +} diff --git a/ref-filter.h b/ref-filter.h index e794b8a676..754038ab07 100644 --- a/ref-filter.h +++ b/ref-filter.h @@ -221,4 +221,7 @@ void filter_is_base(struct repository *r, void ref_filter_init(struct ref_filter *filter); void ref_filter_clear(struct ref_filter *filter); +void ref_format_init(struct ref_format *format); +void ref_format_clear(struct ref_format *format); + #endif /* REF_FILTER_H */ diff --git a/t/t6300-for-each-ref.sh b/t/t6300-for-each-ref.sh index e8db612f95..b3163629c5 100755 --- a/t/t6300-for-each-ref.sh +++ b/t/t6300-for-each-ref.sh @@ -5,6 +5,7 @@ test_description='for-each-ref test' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh GNUPGHOME_NOT_USED=$GNUPGHOME . "$TEST_DIRECTORY"/lib-gpg.sh From patchwork Tue Sep 10 06:57:15 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13798013 Received: from fhigh5-smtp.messagingengine.com (fhigh5-smtp.messagingengine.com [103.168.172.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5B35217B427 for ; Tue, 10 Sep 2024 06:57:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=103.168.172.156 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725951442; cv=none; b=uGJ5XQOgY99fxoyrrzFhwzrmrWCeqsxTqssiuGaLp2Ov9SdfRbKF0sfE75fbNJXrDVKRWMeHoE8KjozFS64Q6mvS8AVa/QYAOQY7WUnGRLhx+oMhB4zTPONK6EpGEo49X3bn9uIHznquPE4nb8wRUzqKCebKSsY9S6fSHNeYhD4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725951442; c=relaxed/simple; bh=eB/mDpVnZOz0f1vG96el4mvQ1kxX5BuvOnNBbmXiNnc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SDuby+vxqWyefV0LWmZF2vBHLVX292if5TVFt6RrFPO7D29+EsxPUXWrZO5jPEz2SbZwimQjZmqz6lbdpKz0I7wWPc75eptf08tWlhi5pCyu2FwWUQvOymjCkZ3Bu/lI8DXrPC96KfOK8DPPy2BLH9Vvr1dvWpNprkb5JAGpNqk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im; spf=pass smtp.mailfrom=pks.im; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b=qg+swm8A; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b=OWiLmtTW; arc=none smtp.client-ip=103.168.172.156 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=pks.im Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pks.im Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=pks.im header.i=@pks.im header.b="qg+swm8A"; dkim=pass (2048-bit key) header.d=messagingengine.com header.i=@messagingengine.com header.b="OWiLmtTW" Received: from phl-compute-09.internal (phl-compute-09.phl.internal [10.202.2.49]) by mailfhigh.phl.internal (Postfix) with ESMTP id 84C3511402B8; Tue, 10 Sep 2024 02:57:19 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-09.internal (MEProxy); Tue, 10 Sep 2024 02:57:19 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm1; t=1725951439; x=1726037839; bh=6BfAQu3dKI ah+5idCk3G6TZeL+avCyO+6AvCcqq7TX0=; b=qg+swm8AUVperTMBlzDnAYfZmn ci/W5N+FaHBn3Yus66oxcj9TLm81ZBWXJbDCHrMhDIuEGYA1SEzLEVUXfK4t3eR3 bdd3OeYOhUnex1V27kVJuPKDIRnfXyDFzxSzRufKn7/cZQqSgla9F9CqI3JeToOv hDoatGOKoc+iFTgxDnhO9vEIOxCUMRLcK8W2Hw6PNezP5n9sXX5Lklew6eYxoC+8 0i7WT3TR1UXcFL5EdvI/ARVB4lXZbvBLWal524zIQYUIBbkYb0upHdHhhfgKIhho RJbg8gLBSoy47BU26KDV9M/nLOInlpBUAqxG6bDBR/6qVlMYip/QQFrMMBPQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1725951439; x=1726037839; bh=6BfAQu3dKIah+5idCk3G6TZeL+av CyO+6AvCcqq7TX0=; b=OWiLmtTW3ZztH19lrTfdpHXX5xomOWy7Eq3cXhUCblQi qTiWe/aCmgcODp3mLobgyy+nkQiIiX/1MFJN93n4zB3SthKdBaRZI2BNxhWuwlPX GNSCnZv6BneAkX3H4P8QZob8OvLCX/klIH0ODKvus4AkqBo9nJ/kpRPwex3vJD3N MvVC6rFDq0t4RAxJ+0GnjzgEJGR9rYeHRcn3QRHbDw93BpKHHK1iIpenbQq+9EOT IRKeVDumnvnIsLNPcXF+UyhRQG+jsF/HHzZjDaM6bhLcIjk6Vm+xwKWfp8ir1ZAS QhEsn8EdoHGmPD//KIX9E6G7NNq5TzCtdUgcbaAprA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddrudeikedgkeeiucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepfffhvfevuffkfhggtggujgesthdtredttddtvden ucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimh eqnecuggftrfgrthhtvghrnhepveekkeffhfeitdeludeigfejtdetvdelvdduhefgueeg udfghfeukefhjedvkedtnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrg hilhhfrhhomhepphhssehpkhhsrdhimhdpnhgspghrtghpthhtohepfedpmhhouggvpehs mhhtphhouhhtpdhrtghpthhtohepghhithesvhhgvghrrdhkvghrnhgvlhdrohhrghdprh gtphhtthhopegsrhhoohhkvgesrghltghhvghmihhsthhsrdhioh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 10 Sep 2024 02:57:18 -0400 (EDT) Received: by vm-mail (OpenSMTPD) with ESMTPSA id 46964f4e (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Tue, 10 Sep 2024 06:57:12 +0000 (UTC) Date: Tue, 10 Sep 2024 08:57:15 +0200 From: Patrick Steinhardt To: git@vger.kernel.org Cc: git@vger.kernel.org, Brooke Kuhlmann Subject: [PATCH 10/9] ref-filter: fix leak with unterminated %(if) atoms Message-ID: <4faf815b780218769520561ecf3abca384a2ee6c.1725951400.git.ps@pks.im> References: <20240909230758.GA921697@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240909230758.GA921697@coredump.intra.peff.net> When parsing `%(if)` atoms we expect a few other atoms to exist to complete it, like `%(then)` and `%(end)`. Whether or not we have seen these other atoms is tracked in an allocated `if_then_else` structure, which gets free'd by the `if_then_else_handler()` once we have parsed the complete conditional expression. This results in a memory leak when the `%(if)` atom is not terminated correctly and thus incomplete. We never end up executing its handler and thus don't end up freeing the structure. Plug this memory leak by introducing a new `at_end_data_free` callback function. If set, we'll execute it in `pop_stack_element()` and pass it the `at_end_data` variable with the intent to free its state. Wire it up for the `%(if)` atom accordingly. Signed-off-by: Patrick Steinhardt --- ref-filter.c | 8 +++++--- t/t6302-for-each-ref-filter.sh | 1 + 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/ref-filter.c b/ref-filter.c index ce1bcfad857..b06e18a569a 100644 --- a/ref-filter.c +++ b/ref-filter.c @@ -1001,6 +1001,7 @@ struct ref_formatting_stack { struct ref_formatting_stack *prev; struct strbuf output; void (*at_end)(struct ref_formatting_stack **stack); + void (*at_end_data_free)(void *data); void *at_end_data; }; @@ -1169,6 +1170,8 @@ static void pop_stack_element(struct ref_formatting_stack **stack) if (prev) strbuf_addbuf(&prev->output, ¤t->output); strbuf_release(¤t->output); + if (current->at_end_data_free) + current->at_end_data_free(current->at_end_data); free(current); *stack = prev; } @@ -1228,15 +1231,13 @@ static void if_then_else_handler(struct ref_formatting_stack **stack) } *stack = cur; - free(if_then_else); } static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state *state, struct strbuf *err UNUSED) { struct ref_formatting_stack *new_stack; - struct if_then_else *if_then_else = xcalloc(1, - sizeof(struct if_then_else)); + struct if_then_else *if_then_else = xcalloc(1, sizeof(*if_then_else)); if_then_else->str = atomv->atom->u.if_then_else.str; if_then_else->cmp_status = atomv->atom->u.if_then_else.cmp_status; @@ -1245,6 +1246,7 @@ static int if_atom_handler(struct atom_value *atomv, struct ref_formatting_state new_stack = state->stack; new_stack->at_end = if_then_else_handler; new_stack->at_end_data = if_then_else; + new_stack->at_end_data_free = free; return 0; } diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh index 163c378cfd1..7f44d3c3f22 100755 --- a/t/t6302-for-each-ref-filter.sh +++ b/t/t6302-for-each-ref-filter.sh @@ -2,6 +2,7 @@ test_description='test for-each-refs usage of ref-filter APIs' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh . "$TEST_DIRECTORY"/lib-gpg.sh