Jeff King <peff@xxxxxxxx> writes: >> but that will no longer be true with the updated code. I'd feel >> safer if at the beginning of the function we create a local variable >> "struct rev_info *repo" that is initialized to opts->revs->repo and >> use it throughout the function instead of the_repository. > > I'm not sure how that makes it any safer, as it would mean using the > suspect repo pointer in more places. Unless you are proposing to do: > > struct repository *r = opts->revs->repo; > if (!r) > r = the_repository; /* or BUG() ? */ Yup, the BUG() variant was exactly what I had in mind, but without the assert, by using opts->revs->repo more consistently, we would make sure nobody will call us with uninitialized opts->revs->repo now or in the future.