On 12/31/2016 07:11 AM, Jeff King wrote: > On Sat, Dec 31, 2016 at 04:12:45AM +0100, Michael Haggerty wrote: >> [...] >> +/* >> + * Callback function for raceproof_create_file(). This function is >> + * expected to do something that makes dirname(path) permanent despite >> + * the fact that other processes might be cleaning up empty >> + * directories at the same time. Usually it will create a file named >> + * path, but alternatively it could create another file in that >> + * directory, or even chdir() into that directory. The function should >> + * return 0 if the action was completed successfully. On error, it >> + * should return a nonzero result and set errno. >> + * raceproof_create_file() treats two errno values specially: >> + * >> + * - ENOENT -- dirname(path) does not exist. In this case, >> + * raceproof_create_file() tries creating dirname(path) >> + * (and any parent directories, if necessary) and calls >> + * the function again. >> + * >> + * - EISDIR -- the file already exists and is a directory. In this >> + * case, raceproof_create_file() deletes the directory >> + * (recursively) if it is empty and calls the function >> + * again. > > It took me a minute to figure out why EISDIR is recursive. > > If we are trying to create "foo/bar/baz", we can only get EISDIR when > "baz" exists and is a directory. I at first took your recursive to me > that we delete it and "foo/bar" and "foo". Which is just silly and > counterproductive. > > But presumably you mean that we delete "foo/bar/baz/xyzzy", etc, up to > "foo/bar/baz", provided they are all empty directories. I think your > comment is probably OK and I was just being thick, but maybe stating it > like: > > ...removes the directory if it is empty (and recursively any empty > directories it contains) and calls the function again. > > would be more clear. That is still leaving the definition of "empty" > implied, but it's hopefully obvious from the context. Yes, that's clearer. I'll change it. Thanks! > [...] Michael