Re: [PATCH] Make git-clean a builtin

Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
From: Jeff King
Date: Sunday, October 7, 2007 - 7:04 pm

On Sat, Oct 06, 2007 at 03:54:06PM -0500, Shawn Bohrer wrote:


This doesn't always put you back where you started, due to symlinks. For
example:

cat >foo.c <<'EOF'
int main(int argc, char **argv) {
  chdir(argv[1]);
  chdir("..");
  execlp("ls", 0);
}
EOF
gcc -o foo foo.c
ln -s /tmp sub
./foo sub

will show that you end up in the root directory.  Something like this is
more robust:

  fd = open(".", O_RDONLY);
  chdir(path);
  ...
  fchdir(fd);

In general, you shouldn't end up there because you don't actually
recurse for symlinks, but there is a race condition (and losing it means
you start recursively removing unintended directories -- oops).

On top of which, this line from the same function isn't very portable:

+                       if (dir->d_type == DT_DIR)

since POSIX specifies nothing but dir->d_name (Solaris, for example,
doesn't define d_type). You need to stat the file.

All of that being said, I think a lot of this is already done in
dir.[ch]. At the very least, you should be able to use
remove_dir_recursively, and for bonus points you can get rid of the
start_command call to ls-files by just walking the dir tree yourself.
I don't know if the latter is required, but it's nice when the
C-ification actually cleans up a bit and uses the internal C interfaces
(which are more efficient and often more clear to read) rather than just
converting shell to C.

-Peff
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
[PATCH] Make git-clean a builtin, Shawn Bohrer, (Sat Oct 6, 1:54 pm)
Re: [PATCH] Make git-clean a builtin, Frank Lichtenheld, (Sat Oct 6, 2:52 pm)
Re: [PATCH] Make git-clean a builtin, Shawn Bohrer, (Sat Oct 6, 6:13 pm)
Re: [PATCH] Make git-clean a builtin, Jeff King, (Sun Oct 7, 7:04 pm)
Re: [PATCH] Make git-clean a builtin, Jeff King, (Sun Oct 7, 7:08 pm)
Re: [PATCH] Make git-clean a builtin, Linus Torvalds, (Sun Oct 7, 7:17 pm)
Re: [PATCH] Make git-clean a builtin, Jeff King, (Sun Oct 7, 7:22 pm)
Re: [PATCH] Make git-clean a builtin, Johannes Sixt, (Sun Oct 7, 11:37 pm)
Re: [PATCH] Make git-clean a builtin, Linus Torvalds, (Mon Oct 8, 11:27 am)