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

[Junio C Hamano <gitster@pobox.com>]

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

> +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...

> +	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.