Re: [WIP v1 2/4] mv: add check_dir_in_index() and solve general dir check issue

[Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>]

On Thu, Mar 31, 2022 at 6:30 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Thu, Mar 31 2022, Shaoxuan Yuan wrote:
>
> > +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)

Thanks for the styling reminders! I should go back and reread CodingGuidelines
more often... Things just slipped off my mind since I'm still trying to remember
the guideline...

-- 
Thanks & Regards,
Shaoxuan