Re: [PATCH 1/2] resolve symlinks when creating lockfiles

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 25, 2007, 23:35 UTC
Message-ID: <7vbqe0cazy.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <11853821951367-git-send-email-bradford.carl.smith@gmail.com>

This probably is going in the right direction, but the code is too densely formatted and unreviewable. Please imitate the layout convention of the other parts of the code.

Show 6 quoted lines

> +/**
> + * p = absolute or relative path name
> + *
> + * Return a pointer into p showing the beginning of the last path name
> + * element.  If p is empty or the root directory ("/"), just return p.
> + */

	/*
         * multi-line comments look like this without the extra
         * asterisk at the beginning of the first line.
         */

> +static char * last_path_elm(char * p)

char *last_path_elem(char *p)

Show 5 quoted lines

> +{
> +	int	p_len = strlen(p);
> +	char *	r;
> +
> +	if (p_len < 1) return p;

        char *r;
	int p_len = strlen(p);

        if (p_len < 1)
                return p;

Aren't p and r of type "const char *", I wonder...

> +	/* r points to last non-null character in p */
> +	r = p + p_len - 1;
> +	/* first skip any trailing slashes */
> +	while (*r == '/' && r > p) r--;

That is

	r = strrchr(p, '/');

isn't it?

Show 6 quoted lines

> +/**
> + * p = char array containing path to existing file or symlink
> + * s = size of p
> + *
> + * If p indicates a valid symlink to an existing file, overwrite p with
> + * the path to the real file.  Otherwise, leave p unmodified.

I suspect some callers use lockfile interface to create a new file. There will be a symlink to not-yet-created real file, that is.