| Message ID | patch-3.4-b7fb5d5a56-20210714T172251Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add a test mode for SANITIZE=leak, run it in CI | expand |
On 14/07/2021 19:23, Ævar Arnfjörð Bjarmason wrote: > Fix a memory leak in a2ba162cda (object-info: support for retrieving > object info, 2021-04-20) which appears to have been based on a > misunderstanding of how the pkt-line.c API works, there is no need to > strdup() input to, it's just a printf()-like format function. > > This fixes a potentially large memory leak, since the number of OID > lines the "object-info" call can be arbitrarily large (or a small one > if the request is small). > > Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> > --- > protocol-caps.c | 5 +++-- > t/t5701-git-serve.sh | 1 + > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/protocol-caps.c b/protocol-caps.c > index 13a9e63a04..901b6795e4 100644 > --- a/protocol-caps.c > +++ b/protocol-caps.c > @@ -69,9 +69,10 @@ static void send_info(struct repository *r, struct packet_writer *writer, > } > } > > - packet_writer_write(writer, "%s", > - strbuf_detach(&send_buffer, NULL)); > + packet_writer_write(writer, "%s", send_buffer.buf); > + strbuf_reset(&send_buffer); > } > + strbuf_release(&send_buffer); > } Good catch! strbuf's seem to be a common source of leak, where either the release is forgotten or detach is used incorrectly - and I'm tempted to try and implement some automated checks to catch those (I wonder if coccicheck is powerful enough for this?). > ...
On Wed, Jul 14, 2021 at 07:23:53PM +0200, Ævar Arnfjörð Bjarmason wrote: > Fix a memory leak in a2ba162cda (object-info: support for retrieving > object info, 2021-04-20) which appears to have been based on a > misunderstanding of how the pkt-line.c API works, there is no need to > strdup() input to, it's just a printf()-like format function. > > This fixes a potentially large memory leak, since the number of OID > lines the "object-info" call can be arbitrarily large (or a small one > if the request is small). Very nice. This will also be much more efficient, since we get to reuse the same buffer in each run through the loop. -Peff
diff --git a/protocol-caps.c b/protocol-caps.c index 13a9e63a04..901b6795e4 100644 --- a/protocol-caps.c +++ b/protocol-caps.c @@ -69,9 +69,10 @@ static void send_info(struct repository *r, struct packet_writer *writer, } } - packet_writer_write(writer, "%s", - strbuf_detach(&send_buffer, NULL)); + packet_writer_write(writer, "%s", send_buffer.buf); + strbuf_reset(&send_buffer); } + strbuf_release(&send_buffer); } int cap_object_info(struct repository *r, struct strvec *keys, diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh index d58efb0aa9..e2f4832adf 100755 --- a/t/t5701-git-serve.sh +++ b/t/t5701-git-serve.sh @@ -5,6 +5,7 @@ test_description='test protocol v2 server commands' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +GIT_TEST_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'test capability advertisement' '
Fix a memory leak in a2ba162cda (object-info: support for retrieving object info, 2021-04-20) which appears to have been based on a misunderstanding of how the pkt-line.c API works, there is no need to strdup() input to, it's just a printf()-like format function. This fixes a potentially large memory leak, since the number of OID lines the "object-info" call can be arbitrarily large (or a small one if the request is small). Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- protocol-caps.c | 5 +++-- t/t5701-git-serve.sh | 1 + 2 files changed, 4 insertions(+), 2 deletions(-)