Re: [WIP v1 2/4] mv: add check_dir_in_index() and solve general dir check issue
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Mar 31, 2022, 10:25 UTC
- Message-ID
- <220331.86mth6qus1.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <20220331091755.385961-3-shaoxuan.yuan02@gmail.com>
On Thu, Mar 31 2022, Shaoxuan Yuan wrote:
Show 7 quoted lines
> +static int check_dir_in_index(const char *dir)
> +{
> + int ret = 0;
> + int length = sizeof(dir) + 1;
> + char *substr = malloc(length);
> +
> + for (int i = 0; i < the_index.cache_nr; i++) {See https://lore.kernel.org/git/xmqqy20r3rv7.fsf@gitster.g/ for how we're not quite using this syntax yet.
This should also be "unsigned int" to go with the "cache_nr" member.
> + memcpy(substr, the_index.cache[i]->name, length);
> + memset(substr + length - 1, 0, 1);
> +
> + if (strcmp(dir, substr) == 0) {Style: don't compare against == 0, or == NULL, use !, see CodingGuidelines.
> + else if (check_dir_in_index(src_w_slash) &&
> + !path_in_sparse_checkout(src_w_slash, &the_index)) {Funny indentation, the ! should be aligned with "(".> - modes[i] = WORKING_DIRECTORY; > + if (!modes[i]) > + modes[i] = WORKING_DIRECTORY;
This works, but assuming things about enum values (even if 0) always seems a bit nasty, can this be a comparison to BOTH instead of !? May or may not be better...
But then again we do xcalloc() to allocate it, so we assume that already, nevermind... :)
(there were also indentation issues below)