From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 24 Aug 2020 17:54:05 GMT
Subject: Re: [PATCH v3 4/4] submodule: port submodule subcommand 'summary' from shell to C
Message-ID: <xmqqo8n053r6.fsf@gitster.c.googlers.com>
In-Reply-To: <20200812194404.17028-5-shouryashukla.oo@gmail.com>

(A few miniscule things I noticed that are irrelevant to the code
structure discussion).

> +static char* verify_submodule_committish(const char *sm_path,

Style: in C, asterisk sticks to the identifier, not type, i.e.

    static char *verify_submodule_committish(const char *sm_path, ...);

> +static void print_submodule_summary(struct summary_cb *info, char* errmsg,

Likewise; "char *errmsg".

> +static void generate_submodule_summary(struct summary_cb *info,
> +				       struct module_cb *p)
> +{
> +	char *displaypath, *src_abbrev, *dst_abbrev;
> +	int missing_src = 0, missing_dst = 0;
> +	char *errmsg = NULL;
> +	int total_commits = -1;
> +
> +	if (!info->cached && oideq(&p->oid_dst, &null_oid)) {
> +		if (S_ISGITLINK(p->mod_dst)) {
> +...
> +		} else {
> +			/* for a submodule removal (mode:0000000), don't warn */
> +			if (p->mod_dst)
> +				warning(_("unexpected mode %d\n"), p->mod_dst);
> +		}
> +	}

Nobody can read mode bits written in decimal.  Use "%o" instead,
perhaps?

Thanks.