Re: [PATCH] Make git-clean a builtin

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