Re: [PATCH 01/23] contrib/coccinnelle: add equals-null.cocci
- From
Carlo Marcelo Arenas Belón <carenas@gmail.com>
- Date
- May 2, 2022, 11:07 UTC
- Message-ID
- <20220502110705.gfdr65dondyigdik@carlos-mbp.lan>
- In-Reply-To
- <xmqqmtg25cjw.fsf@gitster.g>
On Sat, Apr 30, 2022 at 05:20:03PM -0700, Junio C Hamano wrote:
Show 9 quoted lines
> diff -u -p a/compat/mkdir.c b/compat/mkdir.c
> --- a/compat/mkdir.c
> +++ b/compat/mkdir.c
> @@ -9,7 +9,7 @@ int compat_mkdir_wo_trailing_slash(const
> size_t len = strlen(dir);
>
> if (len && dir[len-1] == '/') {
> - if ((tmp_dir = strdup(dir)) == NULL)
> + if (!(tmp_dir = strdup(dir)))did not check them all, but wanted to raise that this specific case, seems to be regressing in readability.
sure; it fails another rule of "not doing assignments inside if", but maybe when it was introduced with 0539ecfdfce (compat: some mkdir() do not like a slash at the end, 2012-08-24), that wasn't valid or it was considered ok for code in compat.
but more importantly it serves as an example of why most of those rules use "try"; after all the assignment WITH the parenthesis and == is clear enough that might not warrant the need of two lines, but without the == it is likely to cause trouble or confuse someone.
Carlo