Re: [PATCH v2] archive: support filtering paths with glob

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 22, 2014, 19:15 UTC
Message-ID: <xmqq4mvzlb5c.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1411271706-1151-1-git-send-email-pclouds@gmail.com>

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

Show 26 quoted lines

> +static void queue_directory(const unsigned char *sha1,
> +		const char *base, int baselen, const char *filename,
> +		unsigned mode, int stage, struct archiver_context *c)
> +{
> +	struct directory *d;
> +	d = xmallocz(sizeof(*d) + baselen + 1 + strlen(filename));
> +	d->up	   = c->bottom;
> +	d->baselen = baselen;
> +	d->mode	   = mode;
> +	d->stage   = stage;
> +	c->bottom  = d;
> +	d->len = sprintf(d->path, "%.*s%s/", baselen, base, filename);
> +	hashcpy(d->sha1, sha1);
> +}
> +
> +static int write_directory(struct archiver_context *c)
> +{
> +	struct directory *d = c->bottom;
> +	int ret;
> +
> +	if (!d)
> +		return 0;
> +	c->bottom = d->up;
> +	d->path[d->len - 1] = '\0'; /* no trailing slash */
> +	ret =
> +		write_directory(c) ||

By recursing into itself using c->bottom set to one level higher, we first flush all the leading directories.

> +		write_archive_entry(d->sha1, d->path, d->baselen,
> +				    d->path + d->baselen, d->mode,
> +				    d->stage, c) != READ_TREE_RECURSIVE;

And after the entries for the leading directories, we write an entry for this directory.

Which makes sense; presumably the caller will then write an entry that hangs this directory after calling this.

When we have a/b/c and a/d/e to be written, the first round would write a/ and then a/b/ with the above, and presumably elsewhere somebody will write a/b/c; next time around we do need to write a/d/ but we wouldn't want to write a/ itself. How is this code preventing the recursion going all the way up every time to avoid repeating a/?

Puzzled...

Show 13 quoted lines

> +	free(d);
> +	return ret ? -1 : 0;
> +}
> +
> +static int queue_or_write_archive_entry(const unsigned char *sha1,
> +		const char *base, int baselen, const char *filename,
> +		unsigned mode, int stage, void *context)
> +{
> +	struct archiver_context *c = context;
> +
> +	while (c->bottom &&
> +	       !(baselen >= c->bottom->len &&
> +		 !strncmp(base, c->bottom->path, c->bottom->len))) {

Is this comparison, which does not see if the substring matches from the beginning but without making sure that the tail of the shorter substring coincides with the directory boundary in the longer string, correctly done? Or does it consider abcd/ef is inside abc/?

Ah, we should be OK as I think <base, baselen> is passed with a trailing slash appended in read_tree_1() when it recurses, so this comparison is correct.