Re: git: 610319c766e9 - main - ufs_vnops.c: newparent is not bool

On Mon, Jul 28, 2025 at 08:57:21PM +0000, Konstantin Belousov wrote:
> The branch main has been updated by kib:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=610319c766e941de96e52f2d28fea9f8cfc51aeb
> 
> commit 610319c766e941de96e52f2d28fea9f8cfc51aeb
> Author:     Konstantin Belousov <kib@FreeBSD.org>
> AuthorDate: 2025-07-27 13:50:57 +0000
> Commit:     Konstantin Belousov <kib@FreeBSD.org>
> CommitDate: 2025-07-28 20:57:14 +0000
> 
>     ufs_vnops.c: newparent is not bool
>     
>     Use proper comparision operators when we need to see if newparent was
>     set to not-zero value.
>     
>     Reviewed by:    mckusick, olce
>     Sponsored by:   The FreeBSD Foundation
>     MFC after:      1 week
>     Differential revision:  https://reviews.freebsd.org/D51573
> ---
>  sys/ufs/ufs/ufs_vnops.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/sys/ufs/ufs/ufs_vnops.c b/sys/ufs/ufs/ufs_vnops.c
> index 406b8f943077..2757fb066981 100644
> --- a/sys/ufs/ufs/ufs_vnops.c
> +++ b/sys/ufs/ufs/ufs_vnops.c
> @@ -1476,7 +1476,7 @@ relock:
>  	 * the user must have write permission in the source so
>  	 * as to be able to change "..".
>  	 */
> -	if (doingdirectory && newparent) {
> +	if (doingdirectory && newparent != 0) {
>  		error = VOP_ACCESS(fvp, VWRITE, tcnp->cn_cred, curthread);
>  		if (error)
>  			goto unlockout;
> @@ -1539,7 +1539,7 @@ relock:
>  	if (tip == NULL) {
>  		if (ITODEV(tdp) != ITODEV(fip))
>  			panic("ufs_rename: EXDEV");
> -		if (doingdirectory && newparent) {
> +		if (doingdirectory && newparent != 0) {
>  			/*
>  			 * Account for ".." in new directory.
>  			 * When source and destination have the same
> @@ -1632,7 +1632,7 @@ relock:
>  			goto bad;
>  		}
>  		if (doingdirectory) {
> -			if (!newparent) {
> +			if (newparent == 0) {
>  				tdp->i_effnlink--;
>  				if (DOINGSOFTDEP(tdvp))
>  					softdep_change_linkcnt(tdp);
> @@ -1642,11 +1642,10 @@ relock:
>  				softdep_change_linkcnt(tip);
>  		}
>  		error = ufs_dirrewrite(tdp, tip, fip->i_number,
> -		    IFTODT(fip->i_mode),
> -		    (doingdirectory && newparent) ? newparent : doingdirectory);
> +		    IFTODT(fip->i_mode), doingdirectory);

Is this part of the change correct?

syzbot is reporting some panics after this change and commit
c069ca085bd185eda4a90dc4bc2b76cceb74579d:
https://syzkaller.appspot.com/bug?extid=18722c8e4008048efb51
https://syzkaller.appspot.com/bug?extid=6a4ea1e13f4e07369785
https://syzkaller.appspot.com/bug?extid=5cb82352555d5d505640
https://syzkaller.appspot.com/bug?extid=602fb6ee1a39abfd3b5c
https://syzkaller.appspot.com/bug?extid=fb35cce6a6f5075a6692
https://syzkaller.appspot.com/bug?extid=6fb8cb919cc686d1a1d0
https://syzkaller.appspot.com/bug?extid=98c39c45a437812f7683
https://syzkaller.appspot.com/bug?extid=02cb048d48b51bcd9c41

>  		if (error) {
>  			if (doingdirectory) {
> -				if (!newparent) {
> +				if (newparent == 0) {
>  					tdp->i_effnlink++;
>  					if (DOINGSOFTDEP(tdvp))
>  						softdep_change_linkcnt(tdp);
> @@ -1669,7 +1668,7 @@ relock:
>  			 * disk, so when running with that code we avoid doing
>  			 * them now.
>  			 */
> -			if (!newparent) {
> +			if (newparent == 0) {
>  				tdp->i_nlink--;
>  				DIP_SET_NLINK(tdp, tdp->i_nlink);
>  				UFS_INODE_SET_FLAG(tdp, IN_CHANGE);
> @@ -1698,7 +1697,7 @@ relock:
>  	 * parent directory must be decremented
>  	 * and ".." set to point to the new parent.
>  	 */
> -	if (doingdirectory && newparent) {
> +	if (doingdirectory && newparent != 0) {
>  		/*
>  		 * Set the directory depth based on its new parent.
>  		 */