diff mbox series

[v2,1/4] ref: initialize "fsck_ref_report" with zero

Message ID Zs351iV2HbdhNvEz@ArchLinux (mailing list archive)
State Superseded
Headers show
Series add ref content check for files backend | expand

Commit Message

shejialuo Aug. 27, 2024, 4:07 p.m. UTC
In "fsck.c::fsck_refs_error_function", we need to tell whether "oid" and
"referent" is NULL. So, we need to always initialize these parameters to
NULL instead of letting them point to anywhere when creating a new
"fsck_ref_report" structure.

The original code explicitly specifies the ".path" field to initialize
the "fsck_ref_report" structure. However, it introduces confusion how we
initialize the other fields. In order to avoid this, initialize the
"fsck_ref_report" with zero to make clear that everything in
"fsck_ref_report" is zero initialized.

Mentored-by: Patrick Steinhardt <ps@pks.im>
Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Signed-off-by: shejialuo <shejialuo@gmail.com>
---
 refs/files-backend.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Junio C Hamano Aug. 27, 2024, 5:49 p.m. UTC | #1
shejialuo <shejialuo@gmail.com> writes:

> The original code explicitly specifies the ".path" field to initialize
> the "fsck_ref_report" structure. However, it introduces confusion how we
> initialize the other fields.

The above description is a bit too strong than what this patch is
actually fixing.  If you explicitly initialize any member of an
aggregate type, other members not mentioned will be implicitly
0-initialized, so the original does not give any confusion to
readers who know what they are reading.

What the patch improves is that the common idiom used in this
code base (and possibly elsewhere) is to use "{ 0 }", instead
of explicitly saying "this particular member is 0-initialized".

    The original code explicitly initializes the "path" member in
    the "struct fsck_ref_report" to NULL (which implicitly
    0-initializes other members in the struct).  It is more
    customary to use "{ 0 }" to express that we are 0-initializing
    everything.

The patch is correct, but spelling it like "{ 0 }" with a space on both
sides is more common [*], and because this patch is all about making it
more idiomatic, let's write it that way.

Thanks.

[Footnote]

 * "git grep -e '{0};' -e '{ 0 };' '*.[ch]'" tells us so.


> Mentored-by: Patrick Steinhardt <ps@pks.im>
> Mentored-by: Karthik Nayak <karthik.188@gmail.com>
> Signed-off-by: shejialuo <shejialuo@gmail.com>
> ---
>  refs/files-backend.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 8d6ec9458d..d6fc3bd67c 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -3446,7 +3446,7 @@ static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
>  		goto cleanup;
>  
>  	if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) {
> -		struct fsck_ref_report report = { .path = NULL };
> +		struct fsck_ref_report report = {0};
>  
>  		strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path);
>  		report.path = sb.buf;
diff mbox series

Patch

diff --git a/refs/files-backend.c b/refs/files-backend.c
index 8d6ec9458d..d6fc3bd67c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3446,7 +3446,7 @@  static int files_fsck_refs_name(struct ref_store *ref_store UNUSED,
 		goto cleanup;
 
 	if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) {
-		struct fsck_ref_report report = { .path = NULL };
+		struct fsck_ref_report report = {0};
 
 		strbuf_addf(&sb, "%s/%s", refs_check_dir, iter->relative_path);
 		report.path = sb.buf;