On Thu, Mar 31, 2022 at 6:30 PM Ævar Arnfjörð Bjarmason
<avarab@xxxxxxxxx> 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
