I don't know much about bash's source code so I cannot comment much. And this kind of arguments are quite opinion based which are not simple yes/no questions. And I believe one thing - the world is not perfect. :)
-clark On Tue, Mar 6, 2018 at 8:26 AM, don fong <df...@dfong.com> wrote: > Clark, > > Just took a look at the code and it is an int: > > > declaring boolean quantities as int is a common practice in old C code. > indeed, all the boolean vars in this program seem to be declared as int. > at least, i don't see anything declared as bool. > > declared type notwithstanding, in the context of subst.c, check_nullness > is being used as a boolean. unfortunately, in retrospect the snippets i > posted are not as clear as i had thought. (is there a way to provide a URL > for subst.c that links back to the devel branch at git.savannah.gnu.org? > as can be done with github or gitlab.) > > consider: > > * there is only one line where check_nullness is set to a non-zero value. > * that line is executed at most once. > * it does check_nullness++ which is another common practice for setting > a boolean variable in C code. > > 8449 check_nullness++; > > * there are at least 2 other references to check_nullness in a boolean > context: > > 8689 if (check_nullness) > > 8687 var_is_null = check_nullness && (var_is_set == 0 || *temp == 0); > > while these boolean-style references may not prove that check_nullness > is boolean, it at least shows that using it as a boolean is not > inconsistent > with existing practices. > > if you agree it's a boolean, wouldn't it be better to test its > truth/falsity > directly instead of comparing it to false (0)? the latter seems like a > typical "trap" that coding pundits warn against. > > consistency would also argue for using the same variable name, > check_nullness > (as in my patch) instead of check_null (as in the altered version). >