| Message ID | ZsIM0L72bei9Fudt@ArchLinux (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | add ref content check for files backend | expand |
shejialuo <shejialuo@gmail.com> writes: > 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 above is correct, but ... > if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) { > - struct fsck_ref_report report = { .path = NULL }; > + struct fsck_ref_report report = FSCK_REF_REPORT_DEFAULT; ... the code without this patch is already doing so. When designated initializers are used to initialize a struct, all members that are not initialized explicitly are implicitly initialized the same as for objects that have static storage duration (meaning: pointers are initialized to NULL, arithmetics are initialized to zero). So I do not quite see why this change is needed. By hiding the fact that the "report" structure is zero-initialized behind the macro, it makes it less obvious that we are clearing everything. If the patch were to rewrite the above like so: struct fsck_ref_report report = { 0 } it would make it even more clear that everything is zero initialized, and also makes it obvious that .path member is not any special. Thanks.
On Tue, Aug 20, 2024 at 09:25:56AM -0700, Junio C Hamano wrote: > > if (check_refname_format(iter->basename, REFNAME_ALLOW_ONELEVEL)) { > > - struct fsck_ref_report report = { .path = NULL }; > > + struct fsck_ref_report report = FSCK_REF_REPORT_DEFAULT; > > ... the code without this patch is already doing so. > > When designated initializers are used to initialize a struct, all > members that are not initialized explicitly are implicitly > initialized the same as for objects that have static storage > duration (meaning: pointers are initialized to NULL, arithmetics are > initialized to zero). > > So I do not quite see why this change is needed. By hiding the fact > that the "report" structure is zero-initialized behind the macro, it > makes it less obvious that we are clearing everything. > > If the patch were to rewrite the above like so: > > struct fsck_ref_report report = { 0 } > > it would make it even more clear that everything is zero > initialized, and also makes it obvious that .path member is not any > special. > Yes, I should use this way. Thanks. > Thanks.
diff --git a/fsck.h b/fsck.h index 500b4c04d2..8894394d16 100644 --- a/fsck.h +++ b/fsck.h @@ -152,6 +152,12 @@ struct fsck_ref_report { const char *referent; }; +#define FSCK_REF_REPORT_DEFAULT { \ + .path = NULL, \ + .oid = NULL, \ + .referent = NULL, \ +} + struct fsck_options { fsck_walk_func walk; fsck_error error_func; diff --git a/refs/files-backend.c b/refs/files-backend.c index 8d6ec9458d..725a4f52e3 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 = FSCK_REF_REPORT_DEFAULT; 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. In order to conveniently create a new "fsck_ref_report", add a new macro "FSCK_REF_REPORT_DEFAULT". Mentored-by: Patrick Steinhardt <ps@pks.im> Mentored-by: Karthik Nayak <karthik.188@gmail.com> Signed-off-by: shejialuo <shejialuo@gmail.com> --- fsck.h | 6 ++++++ refs/files-backend.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)