[PATCH] rm: reuse strbuf for all remove_dir_recursively() calls, again

Subsystems: the rest

STALE3488d

2 messages, 2 authors, 2017-02-13 · open the first message on its own page

[PATCH] rm: reuse strbuf for all remove_dir_recursively() calls, again

From: René Scharfe <hidden>
Date: 2017-02-11 19:51:20

Don't throw the memory allocated for remove_dir_recursively() away after
a single call, use it for the other entries as well instead.

This change was done before in deb8e15a (rm: reuse strbuf for all
remove_dir_recursively() calls), but was reverted as a side-effect of
55856a35 (rm: absorb a submodules git dir before deletion). Reinstate
the optimization.

Signed-off-by: Rene Scharfe <redacted>
---
Was deb8e15a a rebase victim?

 builtin/rm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 452170a3ab..fb79dcab18 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -360,15 +360,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 	 */
 	if (!index_only) {
 		int removed = 0, gitmodules_modified = 0;
+		struct strbuf buf = STRBUF_INIT;
 		for (i = 0; i < list.nr; i++) {
 			const char *path = list.entry[i].name;
 			if (list.entry[i].is_submodule) {
-				struct strbuf buf = STRBUF_INIT;
-
+				strbuf_reset(&buf);
 				strbuf_addstr(&buf, path);
 				if (remove_dir_recursively(&buf, 0))
 					die(_("could not remove '%s'"), path);
-				strbuf_release(&buf);
 
 				removed = 1;
 				if (!remove_path_from_gitmodules(path))
@@ -382,6 +381,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 			if (!removed)
 				die_errno("git rm: '%s'", path);
 		}
+		strbuf_release(&buf);
 		if (gitmodules_modified)
 			stage_updated_gitmodules();
 	}
-- 
2.11.1

Re: [PATCH] rm: reuse strbuf for all remove_dir_recursively() calls, again

From: Stefan Beller <hidden>
Date: 2017-02-13 21:03:18

On Sat, Feb 11, 2017 at 11:51 AM, René Scharfe [off-list ref] wrote:
Don't throw the memory allocated for remove_dir_recursively() away after
a single call, use it for the other entries as well instead.

This change was done before in deb8e15a (rm: reuse strbuf for all
remove_dir_recursively() calls), but was reverted as a side-effect of
55856a35 (rm: absorb a submodules git dir before deletion). Reinstate
the optimization.

Signed-off-by: Rene Scharfe <redacted>
---
Was deb8e15a a rebase victim?
(I do not recall it off the top of my head)

That commit was merged at 03f25e85,
Merge branch 'rs/rm-strbuf-optim', but it looks
like it was reverted as part of 55856a35b2
(rm: absorb a submodules git dir before deletion)

Looking through the discussion at
https://public-inbox.org/git/xmqqmvfich2e.fsf@gitster.mtv.corp.google.com/
there was no apparent signs of confusion, but a reroll was promised, that
I cannot find on the list.

Anyway, the patch below looks good to me.

Thanks,
Stefan
quoted hunk
 builtin/rm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/builtin/rm.c b/builtin/rm.c
index 452170a3ab..fb79dcab18 100644
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -360,15 +360,14 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
         */
        if (!index_only) {
                int removed = 0, gitmodules_modified = 0;
+               struct strbuf buf = STRBUF_INIT;
                for (i = 0; i < list.nr; i++) {
                        const char *path = list.entry[i].name;
                        if (list.entry[i].is_submodule) {
-                               struct strbuf buf = STRBUF_INIT;
-
+                               strbuf_reset(&buf);
                                strbuf_addstr(&buf, path);
                                if (remove_dir_recursively(&buf, 0))
                                        die(_("could not remove '%s'"), path);
-                               strbuf_release(&buf);

                                removed = 1;
                                if (!remove_path_from_gitmodules(path))
@@ -382,6 +381,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
                        if (!removed)
                                die_errno("git rm: '%s'", path);
                }
+               strbuf_release(&buf);
                if (gitmodules_modified)
                        stage_updated_gitmodules();
        }
--
2.11.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help