On Tue, Apr 01, 2014 at 05:58:22PM +0200, Michael Haggerty wrote: > /* > - * p = path that may be a symlink > - * s = full size of p > - * > - * If p is a symlink, attempt to overwrite p with a path to the real > - * file or directory (which may or may not exist), following a chain of > - * symlinks if necessary. Otherwise, leave p unmodified. > + * path contains a path that may be a symlink > * > - * This is a best-effort routine. If an error occurs, p will either be > - * left unmodified or will name a different symlink in a symlink chain > - * that started with p's initial contents. > + * If path is a symlink, attempt to overwrite it with a path to the > + * real file or directory (which may or may not exist), following a > + * chain of symlinks if necessary. Otherwise, leave path unmodified. > * > - * Always returns p. > + * This is a best-effort routine. If an error occurs, path will > + * either be left unmodified or will name a different symlink in a > + * symlink chain that started with path's initial contents. > */ > - > -static char *resolve_symlink(char *p, size_t s) > +static void resolve_symlink(struct strbuf *path) > [...] This is not a problem you are introducing, but do we really want resolve_symlink to be best-effort here? What happens if "a" is a symlink to "b", and two processes try to lock "a" simultaneously? If one succeeds, it will take "b.lock". But the other will take "a.lock", and both will think they have the target locked. I suspect we need to recognize ENOENT for cases where we are creating the file for the first time, but it seems like we should bail on locking from any other transient errors. -Peff -- 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