* [PATCH] dir: remove dead code
@ 2013-09-08 6:09 Ramkumar Ramachandra
2013-09-08 12:30 ` René Scharfe
0 siblings, 1 reply; 3+ messages in thread
From: Ramkumar Ramachandra @ 2013-09-08 6:09 UTC (permalink / raw)
To: Git List
Remove dead code around remove_dir_recursively().
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
dir.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/dir.c b/dir.c
index 910bfcd..2b31241 100644
--- a/dir.c
+++ b/dir.c
@@ -1464,11 +1464,11 @@ int is_empty_dir(const char *path)
return ret;
}
-static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
+int remove_dir_recursively(struct strbuf *path, int flag)
{
DIR *dir;
struct dirent *e;
- int ret = 0, original_len = path->len, len, kept_down = 0;
+ int ret = 0, original_len = path->len, len;
int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
unsigned char submodule_head[20];
@@ -1476,8 +1476,6 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
!resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
/* Do not descend and nuke a nested git work tree. */
- if (kept_up)
- *kept_up = 1;
return 0;
}
@@ -1504,7 +1502,7 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
if (lstat(path->buf, &st))
; /* fall thru */
else if (S_ISDIR(st.st_mode)) {
- if (!remove_dir_recurse(path, flag, &kept_down))
+ if (!remove_dir_recursively(path, flag))
continue; /* happy */
} else if (!only_empty && !unlink(path->buf))
continue; /* happy, too */
@@ -1516,22 +1514,11 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
closedir(dir);
strbuf_setlen(path, original_len);
- if (!ret && !keep_toplevel && !kept_down)
+ if (!ret && !keep_toplevel)
ret = rmdir(path->buf);
- else if (kept_up)
- /*
- * report the uplevel that it is not an error that we
- * did not rmdir() our directory.
- */
- *kept_up = !ret;
return ret;
}
-int remove_dir_recursively(struct strbuf *path, int flag)
-{
- return remove_dir_recurse(path, flag, NULL);
-}
-
void setup_standard_excludes(struct dir_struct *dir)
{
const char *path;
--
1.8.4.100.gde18f6d.dirty
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] dir: remove dead code
2013-09-08 6:09 [PATCH] dir: remove dead code Ramkumar Ramachandra
@ 2013-09-08 12:30 ` René Scharfe
[not found] ` <CALkWK0kYq8nxUVg7bOr-93+WFRSVp4YcpVsJ+3wKcveRV8As2A@mail.gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: René Scharfe @ 2013-09-08 12:30 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
Am 08.09.2013 08:09, schrieb Ramkumar Ramachandra:
> Remove dead code around remove_dir_recursively().
This basically reverts ae2f203e (clean: preserve nested git worktree in
subdirectories). t7300 still seems to pass, though. I wonder why.
>
> Signed-off-by: Ramkumar Ramachandra<artagnon@gmail.com>
> ---
> dir.c | 21 ++++-----------------
> 1 file changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 910bfcd..2b31241 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1464,11 +1464,11 @@ int is_empty_dir(const char *path)
> return ret;
> }
>
> -static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
> +int remove_dir_recursively(struct strbuf *path, int flag)
> {
> DIR *dir;
> struct dirent *e;
> - int ret = 0, original_len = path->len, len, kept_down = 0;
> + int ret = 0, original_len = path->len, len;
> int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
> int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
> unsigned char submodule_head[20];
> @@ -1476,8 +1476,6 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
> if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
> !resolve_gitlink_ref(path->buf, "HEAD", submodule_head)) {
> /* Do not descend and nuke a nested git work tree. */
> - if (kept_up)
> - *kept_up = 1;
> return 0;
> }
>
> @@ -1504,7 +1502,7 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
> if (lstat(path->buf, &st))
> ; /* fall thru */
> else if (S_ISDIR(st.st_mode)) {
> - if (!remove_dir_recurse(path, flag, &kept_down))
> + if (!remove_dir_recursively(path, flag))
kept_down could have been set to 1 here...
> continue; /* happy */
> } else if (!only_empty && !unlink(path->buf))
> continue; /* happy, too */
> @@ -1516,22 +1514,11 @@ static int remove_dir_recurse(struct strbuf *path, int flag, int *kept_up)
> closedir(dir);
>
> strbuf_setlen(path, original_len);
> - if (!ret && !keep_toplevel && !kept_down)
> + if (!ret && !keep_toplevel)
> ret = rmdir(path->buf);
... and would have prevented the rmdir() call here.
Is the removed code really dead? And if not, why does t7300 still pass?
> - else if (kept_up)
> - /*
> - * report the uplevel that it is not an error that we
> - * did not rmdir() our directory.
> - */
> - *kept_up = !ret;
> return ret;
> }
>
> -int remove_dir_recursively(struct strbuf *path, int flag)
> -{
> - return remove_dir_recurse(path, flag, NULL);
> -}
> -
> void setup_standard_excludes(struct dir_struct *dir)
> {
> const char *path;
> -- 1.8.4.100.gde18f6d.dirty
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] dir: remove dead code
[not found] ` <CALkWK0kYq8nxUVg7bOr-93+WFRSVp4YcpVsJ+3wKcveRV8As2A@mail.gmail.com>
@ 2013-09-08 16:03 ` René Scharfe
0 siblings, 0 replies; 3+ messages in thread
From: René Scharfe @ 2013-09-08 16:03 UTC (permalink / raw)
To: Ramkumar Ramachandra; +Cc: Git List
Am 08.09.2013 16:42, schrieb Ramkumar Ramachandra:
> On Sun, Sep 8, 2013 at 6:00 PM, René Scharfe <l.s.r@web.de
> <mailto:l.s.r@web.de>> wrote:
>
> Am 08.09.2013 08:09, schrieb Ramkumar Ramachandra:
>
> Remove dead code around remove_dir_recursively().
>
>
> This basically reverts ae2f203e (clean: preserve nested git worktree
> in subdirectories). t7300 still seems to pass, though. I wonder why.
>
>
> t7300 has nothing to do with ae2f203e.
ae2f203e modified t/t7300-clean.sh.
>
> dir.c | 21 ++++-----------------
> 1 file changed, 4 insertions(+), 17 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 910bfcd..2b31241 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1464,11 +1464,11 @@ int is_empty_dir(const char *path)
> return ret;
> }
>
> -static int remove_dir_recurse(struct strbuf *path, int flag,
> int *kept_up)
> +int remove_dir_recursively(struct strbuf *path, int flag)
> {
> DIR *dir;
> struct dirent *e;
> - int ret = 0, original_len = path->len, len, kept_down = 0;
> + int ret = 0, original_len = path->len, len;
> int only_empty = (flag & REMOVE_DIR_EMPTY_ONLY);
> int keep_toplevel = (flag & REMOVE_DIR_KEEP_TOPLEVEL);
> unsigned char submodule_head[20];
> @@ -1476,8 +1476,6 @@ static int remove_dir_recurse(struct
> strbuf *path, int flag, int *kept_up)
> if ((flag & REMOVE_DIR_KEEP_NESTED_GIT) &&
> !resolve_gitlink_ref(path->__buf, "HEAD",
> submodule_head)) {
> /* Do not descend and nuke a nested git work
> tree. */
> - if (kept_up)
> - *kept_up = 1;
> return 0;
> }
>
> @@ -1504,7 +1502,7 @@ static int remove_dir_recurse(struct
> strbuf *path, int flag, int *kept_up)
> if (lstat(path->buf, &st))
> ; /* fall thru */
> else if (S_ISDIR(st.st_mode)) {
> - if (!remove_dir_recurse(path, flag,
> &kept_down))
> + if (!remove_dir_recursively(path, flag))
>
>
> kept_down could have been set to 1 here...
>
>
> Not possible.
Why?
I guess the answer is that kept_down could have only been set if the
flag REMOVE_DIR_KEEP_NESTED_GIT is given, which only git clean uses,
which in turn has its own implementation of remove_dir_recursively()
named remove_dirs() since f538a91e (git-clean: Display more accurate
delete messages).
That probably means you can remove even more code from
remove_dir_recursively().
>
> continue; /* happy */
> } else if (!only_empty && !unlink(path->buf))
> continue; /* happy, too */
> @@ -1516,22 +1514,11 @@ static int remove_dir_recurse(struct
> strbuf *path, int flag, int *kept_up)
> closedir(dir);
>
> strbuf_setlen(path, original_len);
> - if (!ret && !keep_toplevel && !kept_down)
> + if (!ret && !keep_toplevel)
> ret = rmdir(path->buf);
>
>
> ... and would have prevented the rmdir() call here.
>
> Is the removed code really dead? And if not, why does t7300 still pass?
>
>
> Yes, it clearly is. Shared secret.
What is the secret and who shares it?
Thanks,
René
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-09-08 16:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-08 6:09 [PATCH] dir: remove dead code Ramkumar Ramachandra
2013-09-08 12:30 ` René Scharfe
[not found] ` <CALkWK0kYq8nxUVg7bOr-93+WFRSVp4YcpVsJ+3wKcveRV8As2A@mail.gmail.com>
2013-09-08 16:03 ` René Scharfe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).