Saksham Mittal [off-list ref] writes:
quoted
It is declared, there is an "int i;" a few lines up.
Oh, man, I never even saw that! The patch is completely unnecessary
then. Sorry for that!
No need to say sorry; you'd want to be a bit more careful next time,
that's all.
Also, our code does not introduce a new variable in the first part
of "for (;;)" loop control, so even if the original lacked decl for
"i", the posted patch is not how we write our code for this project.
Thanks.
On Sat, Nov 13 2021, Junio C Hamano wrote:
Saksham Mittal [off-list ref] writes:
quoted
quoted
It is declared, there is an "int i;" a few lines up.
Oh, man, I never even saw that! The patch is completely unnecessary
then. Sorry for that!
No need to say sorry; you'd want to be a bit more careful next time,
that's all.
Also, our code does not introduce a new variable in the first part
of "for (;;)" loop control, so even if the original lacked decl for
"i", the posted patch is not how we write our code for this project.
Just curious: Out of preference, or for compatibility with older C
standards?
I'd think with the things we depend on in C99 it's probable that we
could start using this if standards conformance is the only obstacle.
But I haven't tested, so maybe I'm wrong, I'm just assuming that with
the C99 features we do have a hard dependency on surely anyone
implementing those would have implemented this too.
There's also a stylistic reason to avoid this pattern, i.e. some would
argue that it's better to declare variables up-front, since it tends to
encourage one to keep function definitions smaller (various in-tree
evidence to the contrary, but whatever).
I'd generally agree with that viewpoint & desire, but there's also cases
where being able to declare things in-line helps readability, e.g. when
your function needs two for-loops for some reason, they're set a bit
apart. Then the reader doesn't need to scan for whether an "i" is used
in-between the two.
I was thinking of the below code in bundle.c, I suppose some might find
the post-image less readable, but I remember starting to hunt around for
other out-of-loop uses of "i", which the post-image makes clear could be
avoided as far as variable scoping goes:
diff --git a/bundle.c b/bundle.c
index a0bb687b0f4..94edc186187 100644
--- a/bundle.c
+++ b/bundle.c
@@ -194,14 +194,14 @@ int verify_bundle(struct repository *r,
struct rev_info revs;
const char *argv[] = {NULL, "--all", NULL};
struct commit *commit;
- int i, ret = 0, req_nr;
+ int ret = 0, req_nr;
const char *message = _("Repository lacks these prerequisite commits:");
if (!r || !r->objects || !r->objects->odb)
return error(_("need a repository to verify a bundle"));
repo_init_revisions(r, &revs, NULL);
- for (i = 0; i < p->nr; i++) {
+ for (int i = 0; i < p->nr; i++) {
struct string_list_item *e = p->items + i;
const char *name = e->string;
struct object_id *oid = e->util;@@ -223,12 +223,11 @@ int verify_bundle(struct repository *r,
if (prepare_revision_walk(&revs))
die(_("revision walk setup failed"));
- i = req_nr;
- while (i && (commit = get_revision(&revs)))
+ for (int i = req_nr; i && (commit = get_revision(&revs));)
if (commit->object.flags & PREREQ_MARK)
i--;
- for (i = 0; i < p->nr; i++) {
+ for (int i = 0; i < p->nr; i++) {
struct string_list_item *e = p->items + i;
const char *name = e->string;
const struct object_id *oid = e->util;@@ -242,7 +241,7 @@ int verify_bundle(struct repository *r,
}
/* Clean up objects used, as they will be reused. */
- for (i = 0; i < p->nr; i++) {
+ for (int i = 0; i < p->nr; i++) {
struct string_list_item *e = p->items + i;
struct object_id *oid = e->util;
commit = lookup_commit_reference_gently(r, oid, 1);