| Message ID | patch-2.3-f1fcc31717-20210409T080534Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blob/object.c: trivial readability improvements | expand |
On Fri, Apr 09, 2021 at 10:07:28AM +0200, Ævar Arnfjörð Bjarmason wrote: > Initialize a "struct object obj*" variable to NULL explicitly and > return it instead of leaving it uninitialized until the "while" > loop. > > There was no bug here, it's just less confusing when debugging if the > "obj" is either NULL or a valid object, not some random invalid > pointer. > > [...] > > struct object *lookup_object(struct repository *r, const struct object_id *oid) > { > unsigned int i, first; > - struct object *obj; > + struct object *obj = NULL; > > if (!r->parsed_objects->obj_hash) > - return NULL; > + return obj; I actually prefer the original style (where any "can we bail early" checks just explicitly return NULL, rather than making you check to see that obj is NULL). But it's pretty subjective, and I don't feel strongly. > first = i = hash_obj(oid, r->parsed_objects->obj_hash_size); > while ((obj = r->parsed_objects->obj_hash[i]) != NULL) { The important thing is that "obj" is not used uninitialized, which it isn't (before or after). -Peff
Jeff King <peff@peff.net> writes: >> struct object *lookup_object(struct repository *r, const struct object_id *oid) >> { >> unsigned int i, first; >> - struct object *obj; >> + struct object *obj = NULL; >> >> if (!r->parsed_objects->obj_hash) >> - return NULL; >> + return obj; > > I actually prefer the original style (where any "can we bail early" > checks just explicitly return NULL, rather than making you check to see > that obj is NULL). Perhaps I should return "me too" here.
diff --git a/object.c b/object.c index 63896abf01..7fdca3ed1e 100644 --- a/object.c +++ b/object.c @@ -87,10 +87,10 @@ static void insert_obj_hash(struct object *obj, struct object **hash, unsigned i struct object *lookup_object(struct repository *r, const struct object_id *oid) { unsigned int i, first; - struct object *obj; + struct object *obj = NULL; if (!r->parsed_objects->obj_hash) - return NULL; + return obj; first = i = hash_obj(oid, r->parsed_objects->obj_hash_size); while ((obj = r->parsed_objects->obj_hash[i]) != NULL) {
Initialize a "struct object obj*" variable to NULL explicitly and return it instead of leaving it uninitialized until the "while" loop. There was no bug here, it's just less confusing when debugging if the "obj" is either NULL or a valid object, not some random invalid pointer. See 0556a11a0df (git object hash cleanups, 2006-06-30) for the initial implementation. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- object.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)