From patchwork Thu May 30 06:39:32 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13679806 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 1F8287F for ; Thu, 30 May 2024 06:39:33 +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=1717051176; cv=none; b=Lsxxe7lH4YbABnqUZriHozC6hWNX+C6SUMj/HzkuWZoInWBdo9szuQ8NYdH8XoY9fTid37ncmQ/VGEJAUNpxCarJRvxjBRiHDyQe0k4aoHMbgGLc6lhM+BkM07rctqQNEbdU2DFOulF0Ld+mlcOAZGc9WMQ0XOAE3uLshnHuVvg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717051176; c=relaxed/simple; bh=kCAxMrEVMRlczHBV+hiQv2em8uVAbX/q3/HqQI7rXB8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=fPu41xmV9PYJEQ9bFIJe8FnHkcM3w2dWxefY2gUqS+KlgOtZpllk0e5Rsu44MfhlKpFs2WsiLGuf29bxZM+vsG+90uzaqyCYgLoizo9840vT7On5GJlGIcHaSFilOsHoMXMcnhm5Da/FZw7pl8FjJpNgdn1+6ZI7ol1iQDTm/ww= 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 16754 invoked by uid 109); 30 May 2024 06:39:33 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 30 May 2024 06:39:33 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 27595 invoked by uid 111); 30 May 2024 06:39:30 -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; Thu, 30 May 2024 02:39:30 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 30 May 2024 02:39:32 -0400 From: Jeff King To: Patrick Steinhardt Cc: git@vger.kernel.org, Eric Sunshine , Junio C Hamano , Karthik Nayak Subject: [PATCH 1/5] t-strvec: use va_end() to match va_start() Message-ID: <20240530063932.GA1949704@coredump.intra.peff.net> References: <20240530063857.GA1942535@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: <20240530063857.GA1942535@coredump.intra.peff.net> Our check_strvec_loc() helper uses a variable argument list. When we va_start(), we must be sure to va_end() before leaving the function. This is required by the standard (though the effect of forgetting will vary between platforms). Signed-off-by: Jeff King --- t/unit-tests/t-strvec.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/unit-tests/t-strvec.c b/t/unit-tests/t-strvec.c index f17fb10d9e..6c1465ee61 100644 --- a/t/unit-tests/t-strvec.c +++ b/t/unit-tests/t-strvec.c @@ -22,11 +22,13 @@ static void check_strvec_loc(const char *loc, struct strvec *vec, ...) strbuf_addf(&msg, "strvec index %"PRIuMAX, (uintmax_t) nr); test_assert(loc, msg.buf, 0); strbuf_release(&msg); + va_end(ap); return; } nr++; } + va_end(ap); check_uint(vec->nr, ==, nr); check_uint(vec->alloc, >=, nr); From patchwork Thu May 30 06:39:56 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13679807 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 3C2EA7F for ; Thu, 30 May 2024 06:39:57 +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=1717051200; cv=none; b=ee3O+OcI2PNEGeLKBg3yvQXgEKq2eJWEvZ6YibV8jNh1qwFha4xRHsVe3ekKOgCj+6VIatuITmA7J2q2r+2aaLX8E3EkKb8xu+6dy9RViol9pXQrRDwmN7dQodkOuLxbf9ftvEDU9aZZnjTdixRYq9eVuiglSymFFQy2abnyxSI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717051200; c=relaxed/simple; bh=Gyk/6CBhQupVP+6ELjqE0d1HCDgE2ZaL6kp6+dVB9cE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Laptz9SFBSJpKi3HBP0wWKTg2xtQr0YbFNb5Upi/FYiSbf41js895S54O4orcUO7tBnFGt59TacwJjSft08M1YygjA4GhpI7TlPqgdHWfr52YsTXLItowm9xY3jLE0bTyCUpZLQUOfMdFARNzf1BrZ4g+AnBIoT/eTcKcG59faY= 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 16775 invoked by uid 109); 30 May 2024 06:39:57 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 30 May 2024 06:39:57 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 27606 invoked by uid 111); 30 May 2024 06:39: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; Thu, 30 May 2024 02:39:54 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 30 May 2024 02:39:56 -0400 From: Jeff King To: Patrick Steinhardt Cc: git@vger.kernel.org, Eric Sunshine , Junio C Hamano , Karthik Nayak Subject: [PATCH 2/5] t-strvec: mark variable-arg helper with LAST_ARG_MUST_BE_NULL Message-ID: <20240530063956.GB1949704@coredump.intra.peff.net> References: <20240530063857.GA1942535@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: <20240530063857.GA1942535@coredump.intra.peff.net> This will let the compiler catch a problem like: /* oops, we forgot the NULL */ check_strvec(&vec, "foo"); rather than triggering undefined behavior at runtime. Signed-off-by: Jeff King --- t/unit-tests/t-strvec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/t/unit-tests/t-strvec.c b/t/unit-tests/t-strvec.c index 6c1465ee61..d4615ab06d 100644 --- a/t/unit-tests/t-strvec.c +++ b/t/unit-tests/t-strvec.c @@ -4,6 +4,7 @@ #define check_strvec(vec, ...) \ check_strvec_loc(TEST_LOCATION(), vec, __VA_ARGS__) +LAST_ARG_MUST_BE_NULL static void check_strvec_loc(const char *loc, struct strvec *vec, ...) { va_list ap; From patchwork Thu May 30 06:44:22 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13679808 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 E147C7F for ; Thu, 30 May 2024 06:44:24 +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=1717051466; cv=none; b=Mgn36V5IPwh2wqgmO/xA/ugh+CVswR6+gVbMz5JK/rroTlVC9zAkD+NCC5Tp8tas0CsgXfXrnj9YuWEvqj8EMjdOU32NBgxidY1Kg84JdkWXdbk7FMTHo57QSqSk70KocmaCaLlmyYvXH5cWZ0JEdijtDwkZYtpxm8fL4jMtQgQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717051466; c=relaxed/simple; bh=2kNsVg63SAbl9Zy9Pxypxy3NQ+zHqWhmY2So6KXLIQ0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=bPxHIMOmluid758hGT5P5iAnRZJfSbeuoE6/V1kwV/KdhN3Oid25MHcuUkqCh4TjVtTNl5RxsqaNpNaqMB+6MiHgKKlw4aKqD4WP6Igxnot061gqCY2dJHkF5qbOreH5+mTixEq7pPBtQlsZkSL6r/dy+m2xNwSaRw/rhdC7E8Y= 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 16796 invoked by uid 109); 30 May 2024 06:44:24 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 30 May 2024 06:44:23 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 27639 invoked by uid 111); 30 May 2024 06:44:21 -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; Thu, 30 May 2024 02:44:21 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 30 May 2024 02:44:22 -0400 From: Jeff King To: Patrick Steinhardt Cc: git@vger.kernel.org, Eric Sunshine , Junio C Hamano , Karthik Nayak Subject: [PATCH 3/5] mv: move src_dir cleanup to end of cmd_mv() Message-ID: <20240530064422.GC1949704@coredump.intra.peff.net> References: <20240530063857.GA1942535@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: <20240530063857.GA1942535@coredump.intra.peff.net> Commit b6f51e3db9 (mv: cleanup empty WORKING_DIRECTORY, 2022-08-09) added an auxiliary array where we store directory arguments that we see while processing the incoming arguments. After actually moving things, we then use that array to remove now-empty directories, and then immediately free the array. But if the actual move queues any errors in only_match_skip_worktree, that can cause us to jump straight to the "out" label to clean up, skipping the free() and leaking the array. Let's push the free() down past the "out" label so that we always clean up (the array is initialized to NULL, so this is always safe). We'll hold on to the memory a little longer than necessary, but clarity is more important than micro-optimizing here. Note that the adjacent "a_src_dir" strbuf does not suffer the same problem; it is only allocated during the removal step. Signed-off-by: Jeff King --- Reported as "new" by Coverity, but I think that is only because of the "goto out". Before your recent patches it was a straight "return", which was even worse. ;) builtin/mv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/mv.c b/builtin/mv.c index 81ca910de6..852b4e92c1 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -556,7 +556,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } strbuf_release(&a_src_dir); - free(src_dir); if (dirty_paths.nr) advise_on_moving_dirty_path(&dirty_paths); @@ -571,6 +570,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) ret = 0; out: + free(src_dir); free(dst_w_slash); string_list_clear(&src_for_dst, 0); string_list_clear(&dirty_paths, 0); From patchwork Thu May 30 06:45:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13679809 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 A0E6D143739 for ; Thu, 30 May 2024 06:45:22 +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=1717051524; cv=none; b=KAXH37Xk5ZMKhf1/L91d9PSVui8PG2/xDVHko9ory9J5cJERB0zbMnZRHGZgeuGCd0dYFGIbfOnVZGkCZJl0Q33u0h6J27iYis+Mhd9jp93MuYmJ4k02uV2KWByXGhKPrYPB9yfzxx8jQKBgVwBn7O7hqHarW5J8In/SbYVwd50= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717051524; c=relaxed/simple; bh=FCPomW+MLc5IfG/su35hexOZjGheWoiq1pI83NjxyM4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=V4awqYtpm1XH7PNpFZGV+Z3w9oeHp/Ug5sAJ9ipPAfB22q/GgR6vWB3BKrXrdqGRtGc0+IgbNdVmEPf2e9XOM4r5dAkLULf1uP4w4JRzgUX5dDYKpq1ht1twJJ7sPezoRAbmfSooAaoEEwHLO1rTUipB+NL0LaU5bDHZ5Vq9oto= 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 16808 invoked by uid 109); 30 May 2024 06:45:21 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 30 May 2024 06:45:21 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 27681 invoked by uid 111); 30 May 2024 06:45:19 -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; Thu, 30 May 2024 02:45:19 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 30 May 2024 02:45:21 -0400 From: Jeff King To: Patrick Steinhardt Cc: git@vger.kernel.org, Eric Sunshine , Junio C Hamano , Karthik Nayak Subject: [PATCH 4/5] mv: factor out empty src_dir removal Message-ID: <20240530064521.GD1949704@coredump.intra.peff.net> References: <20240530063857.GA1942535@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: <20240530063857.GA1942535@coredump.intra.peff.net> This pulls the loop added by b6f51e3db9 (mv: cleanup empty WORKING_DIRECTORY, 2022-08-09) into a sub-function. That reduces clutter in cmd_mv() and makes it easier to see that the lifetime of the a_src_dir strbuf is limited to this code (and thus its cleanup doesn't need to go after the "out" label). Another option would be to just declare the strbuf inside the loop, since it is only used there. But this refactor retains the existing property that we can reuse the allocated buffer for each iteration of the loop. That optimization is probably overkill, but I think the sub-function is more readable anyway, and then keeping the optimization is basically free. Signed-off-by: Jeff King --- builtin/mv.c | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 852b4e92c1..01725e4a20 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -156,6 +156,28 @@ static int empty_dir_has_sparse_contents(const char *name) return ret; } +static void remove_empty_src_dirs(const char **src_dir, size_t src_dir_nr) +{ + size_t i; + struct strbuf a_src_dir = STRBUF_INIT; + + for (i = 0; i < src_dir_nr; i++) { + int dummy; + strbuf_addstr(&a_src_dir, src_dir[i]); + /* + * if entries under a_src_dir are all moved away, + * recursively remove a_src_dir to cleanup + */ + if (index_range_of_same_dir(a_src_dir.buf, a_src_dir.len, + &dummy, &dummy) < 1) { + remove_dir_recursively(&a_src_dir, 0); + } + strbuf_reset(&a_src_dir); + } + + strbuf_release(&a_src_dir); +} + int cmd_mv(int argc, const char **argv, const char *prefix) { int i, flags, gitmodules_modified = 0; @@ -177,7 +199,6 @@ int cmd_mv(int argc, const char **argv, const char *prefix) char *dst_w_slash = NULL; const char **src_dir = NULL; int src_dir_nr = 0, src_dir_alloc = 0; - struct strbuf a_src_dir = STRBUF_INIT; enum update_mode *modes, dst_mode = 0; struct stat st, dest_st; struct string_list src_for_dst = STRING_LIST_INIT_DUP; @@ -538,24 +559,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } } - /* - * cleanup the empty src_dirs - */ - for (i = 0; i < src_dir_nr; i++) { - int dummy; - strbuf_addstr(&a_src_dir, src_dir[i]); - /* - * if entries under a_src_dir are all moved away, - * recursively remove a_src_dir to cleanup - */ - if (index_range_of_same_dir(a_src_dir.buf, a_src_dir.len, - &dummy, &dummy) < 1) { - remove_dir_recursively(&a_src_dir, 0); - } - strbuf_reset(&a_src_dir); - } - - strbuf_release(&a_src_dir); + remove_empty_src_dirs(src_dir, src_dir_nr); if (dirty_paths.nr) advise_on_moving_dirty_path(&dirty_paths); From patchwork Thu May 30 06:46:38 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13679812 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 972B73207 for ; Thu, 30 May 2024 06:46:40 +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=1717051602; cv=none; b=nsJC/Vf3X14H1SpegIOtZwfRCr+NBiQeOAIwEDX8JlSUvAFMUQB0puAaFhAWELNJO212Irscws/1QedRDC/aqIztpJ+uWUxl7aGZu9pJb9UWQ+y2FzQPiyhQ56EWy++Gf81a+qBNjQ96/JlkYe64/Pio76ft7hYv30SwAUlVZkE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717051602; c=relaxed/simple; bh=QlauurVVKhlzavNDT07ujeSfoI0lDelrQLMrA8N1BiE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=RLzDWedxXB0tpepq4pZgiI52k4xCnfZ87PPI42lZwo31LbYeuCP2Nv3waSCd2O7l4fDl+CT43vp4zrg8mqwYvnW88uGHxla/ehgm/M8f9fRcLJRlk2WlKczKFGWoRedwCW5yaJsTMydUs3kaneMjQv75fxSgsNGFwulex2fbMmI= 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 16825 invoked by uid 109); 30 May 2024 06:46:39 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Thu, 30 May 2024 06:46:39 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 27689 invoked by uid 111); 30 May 2024 06:46:37 -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; Thu, 30 May 2024 02:46:37 -0400 Authentication-Results: peff.net; auth=none Date: Thu, 30 May 2024 02:46:38 -0400 From: Jeff King To: Patrick Steinhardt Cc: git@vger.kernel.org, Eric Sunshine , Junio C Hamano , Karthik Nayak Subject: [PATCH 5/5] mv: replace src_dir with a strvec Message-ID: <20240530064638.GE1949704@coredump.intra.peff.net> References: <20240530063857.GA1942535@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: <20240530063857.GA1942535@coredump.intra.peff.net> We manually manage the src_dir array with ALLOC_GROW. Using a strvec is a little more ergonomic, and makes the memory ownership more clear. It does mean that we copy the strings (which were otherwise just pointers into the "sources" strvec), but using the same rationale as 9fcd9e4e72 (builtin/mv duplicate string list memory, 2024-05-27), it's just not enough to be worth worrying about here. As a bonus, this gets rid of some "int"s used for allocation management (though in practice these were limited to command-line sizes and thus not overflowable). Signed-off-by: Jeff King --- builtin/mv.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/builtin/mv.c b/builtin/mv.c index 01725e4a20..6c69033c5f 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -197,8 +197,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) struct strvec submodule_gitfiles_to_free = STRVEC_INIT; const char **submodule_gitfiles; char *dst_w_slash = NULL; - const char **src_dir = NULL; - int src_dir_nr = 0, src_dir_alloc = 0; + struct strvec src_dir = STRVEC_INIT; enum update_mode *modes, dst_mode = 0; struct stat st, dest_st; struct string_list src_for_dst = STRING_LIST_INIT_DUP; @@ -344,8 +343,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) /* last - first >= 1 */ modes[i] |= WORKING_DIRECTORY; - ALLOC_GROW(src_dir, src_dir_nr + 1, src_dir_alloc); - src_dir[src_dir_nr++] = src; + strvec_push(&src_dir, src); n = argc + last - first; REALLOC_ARRAY(modes, n); @@ -559,7 +557,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) } } - remove_empty_src_dirs(src_dir, src_dir_nr); + remove_empty_src_dirs(src_dir.v, src_dir.nr); if (dirty_paths.nr) advise_on_moving_dirty_path(&dirty_paths); @@ -574,7 +572,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix) ret = 0; out: - free(src_dir); + strvec_clear(&src_dir); free(dst_w_slash); string_list_clear(&src_for_dst, 0); string_list_clear(&dirty_paths, 0);