On Sat, Oct 06, 2007 at 11:52:53PM +0200, Frank Lichtenheld wrote:
> On Sat, Oct 06, 2007 at 03:54:06PM -0500, Shawn Bohrer wrote:
> > +static int remove_directory(const char *path)
> > +{
> > + DIR *d;
> > + struct dirent *dir;
> > + d = opendir(path);
> > + if (d) {
> > + chdir(path);
> > + while ((dir = readdir(d)) != NULL) {
> > + if(strcmp( dir->d_name, ".") == 0 ||
> > + strcmp( dir->d_name, ".." ) == 0 )
> > + continue;
> > + if (dir->d_type == DT_DIR)
> > + remove_directory(dir->d_name);
> > + else
> > + unlink(dir->d_name);
> > + }
> > + }
> > + closedir(d);
> > + chdir("..");
> > + return rmdir(path);
> > +}
>
> The unconditional chdir(..) after the conditional chdir(path) seems like
> asking for trouble to me...
Agreed, I'm not sure what I was thinking there.
> > + while (fgets(path, sizeof(path), cmd_fout) != NULL) {
> > + struct stat st;
> > + char *p;
> > + p = strrchr(path, '\n');
> > + if ( p != NULL )
> > + *p = '\0';
>
> What happens in case p == NULL? It simply tries to remove the partial
> path?
If p == NULL then the path didn't have a EOL character. This shouldn't
ever really happen since fgets() leaves the EOL character as part of the
string, and it is processing the output of git-ls-files which will
provide one path per line. If it does happen for some reason then
either we will happily remove the file/directory, or if the path is
garbage then we will simply fail to remove anything.
> > + fclose(cmd_fout);
> > + finish_command(&cmd);
> > + if (!ignored && !access(git_path("info/exclude"), F_OK))
> > + free(buf);
>
> There is a race condition here of the value of access() changes between
> the two calls. Not one likely to trigger but it should be easy to avoid
> alltogether.
Yes, though unlikely I agree this wasn't very smart in the first place
so I fixed it and will send reworked patch soon.
--
Shawn
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
