bug-bash
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: coding standards


From: Clark Wang
Subject: Re: coding standards
Date: Tue, 6 Mar 2018 17:15:36 +0800

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 <dfong@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).
>


reply via email to

[Prev in Thread] Current Thread [Next in Thread]