| Message ID | Zs351iV2HbdhNvEz@ArchLinux (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add ref content check for files backend | expand |
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 --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;
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(-)