Re: [WIP v1 2/4] mv: add check_dir_in_index() and solve general dir check issue
- From
Shaoxuan Yuan <shaoxuan.yuan02@gmail.com>
- Date
- Apr 1, 2022, 03:51 UTC
- Message-ID
- <CAJyCBOThDykT9dn09CutAdaSwWXapVn3St3XnnLYmORfGfq+=w@mail.gmail.com>
- In-Reply-To
- <220331.86mth6qus1.gmgdl@evledraar.gmail.com>
On Thu, Mar 31, 2022 at 6:30 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
Show 40 quoted lines
>
> 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