On Wed, Nov 27, 2013 at 7:00 PM, Thomas Gummerer <t.gummerer@xxxxxxxxx> wrote:
> --- a/cache.h
> +++ b/cache.h
> @@ -132,11 +141,17 @@ struct cache_entry {
> char name[FLEX_ARRAY]; /* more */
> };
>
> +#define CE_NAMEMASK (0x0fff)
CE_NAMEMASK is redefined in read-cache-v2.c in "read-cache: move index
v2 specific functions to their own file". My gcc is smart enough to
see the two defines are about the same value and does not warn me. But
we should remove one (likely this one as I see no use of this macro
outside read-cache-v2.c)
> #define CE_STAGEMASK (0x3000)
> #define CE_EXTENDED (0x4000)
> #define CE_VALID (0x8000)
> +#define CE_SMUDGED (0x0400) /* index v5 only flag */
> #define CE_STAGESHIFT 12
>
> +#define CONFLICT_CONFLICTED (0x8000)
> +#define CONFLICT_STAGESHIFT 13
> +#define CONFLICT_STAGEMASK (0x6000)
> +
> /*
> * Range 0xFFFF0000 in ce_flags is divided into
> * two parts: in-memory flags and on-disk ones.
> diff --git a/read-cache-v5.c b/read-cache-v5.c
> new file mode 100644
> index 0000000..9d8c8f0
> --- /dev/null
> +++ b/read-cache-v5.c
> +static int read_index_v5(struct index_state *istate, void *mmap,
> + unsigned long mmap_size, struct filter_opts *opts)
> +{
> + unsigned int entry_offset, foffsetblock, nr = 0, *extoffsets;
> + unsigned int dir_offset, dir_table_offset;
> + int need_root = 0, i;
> + uint32_t *offset;
> + struct directory_entry *root_directory, *de, *last_de;
> + const char **paths = NULL;
> + struct pathspec adjusted_pathspec;
> + struct cache_header *hdr;
> + struct cache_header_v5 *hdr_v5;
> +
> + hdr = mmap;
> + hdr_v5 = ptr_add(mmap, sizeof(*hdr));
> + istate->cache_alloc = alloc_nr(ntohl(hdr->hdr_entries));
> + istate->cache = xcalloc(istate->cache_alloc, sizeof(struct cache_entry *));
> + extoffsets = xcalloc(ntohl(hdr_v5->hdr_nextension), sizeof(int));
> + for (i = 0; i < ntohl(hdr_v5->hdr_nextension); i++) {
> + offset = ptr_add(mmap, sizeof(*hdr) + sizeof(*hdr_v5));
> + extoffsets[i] = htonl(*offset);
> + }
> +
> + /* Skip size of the header + crc sum + size of offsets to extensions + size of offsets */
> + dir_offset = sizeof(*hdr) + sizeof(*hdr_v5) + ntohl(hdr_v5->hdr_nextension) * 4 + 4
> + + (ntohl(hdr_v5->hdr_ndir) + 1) * 4;
> + dir_table_offset = sizeof(*hdr) + sizeof(*hdr_v5) + ntohl(hdr_v5->hdr_nextension) * 4 + 4;
> + root_directory = read_directories(&dir_offset, &dir_table_offset,
> + mmap, mmap_size);
> +
> + entry_offset = ntohl(hdr_v5->hdr_fblockoffset);
> + foffsetblock = dir_offset;
> +
> + if (opts && opts->pathspec && opts->pathspec->nr) {
> + paths = xmalloc((opts->pathspec->nr + 1)*sizeof(char *));
> + paths[opts->pathspec->nr] = NULL;
Put this statement here
GUARD_PATHSPEC(opts->pathspec,
PATHSPEC_FROMTOP |
PATHSPEC_MAXDEPTH |
PATHSPEC_LITERAL |
PATHSPEC_GLOB |
PATHSPEC_ICASE);
This says the mentioned magic is safe in this code. New magic may or
may not be and needs to be checked (soonest by me, I'm going to add
negative pathspec and I'll need to look into how it should be handled
in this code block).
> + for (i = 0; i < opts->pathspec->nr; i++) {
> + char *super = strdup(opts->pathspec->items[i].match);
> + int len = strlen(super);
You should only check as far as items[i].nowildcard_len, not strlen().
The rest could be wildcards and stuff and not so reliable.
> + while (len && super[len - 1] == '/' && super[len - 2] == '/')
> + super[--len] = '\0'; /* strip all but one trailing slash */
> + while (len && super[--len] != '/')
> + ; /* scan backwards to next / */
> + if (len >= 0)
> + super[len--] = '\0';
> + if (len <= 0) {
> + need_root = 1;
> + break;
> + }
> + paths[i] = super;
> + }
And maybe put the comment "FIXME: consider merging this code with
create_simplify() in dir.c" somewhere. It's for me to look for things
to do when I'm bored ;-)
> + }
> +
> + if (!need_root)
> + parse_pathspec(&adjusted_pathspec, PATHSPEC_ALL_MAGIC, PATHSPEC_PREFER_CWD, NULL, paths);
I would go with PATHSPEC_PREFER_FULL instead of _CWD as it's safer.
Looking only at this function without caller context, it's hard to say
if _CWD is the right choice.
> +
> + de = root_directory;
> + last_de = de;
This statement is redundant. last_de is only used in one code block
below and it's always re-initialized before entering the loop to skip
subdirs.
> + while (de) {
> + if (need_root ||
> + match_pathspec_depth(&adjusted_pathspec, de->pathname, de->de_pathlen, 0, NULL)) {
> + if (read_entries(istate, de, entry_offset,
> + mmap, mmap_size, &nr,
> + foffsetblock) < 0)
> + return -1;
> + } else {
> + last_de = de;
> + for (i = 0; i < de->de_nsubtrees; i++) {
> + de->sub[i]->next = last_de->next;
> + last_de->next = de->sub[i];
> + last_de = last_de->next;
> + }
> + }
> + de = de->next;
> + }
> + free_directory_tree(root_directory);
> + istate->cache_nr = nr;
> + return 0;
> +}
--
Duy
--
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