Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 30 Jun 2021 13:49:04 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Amir Goldstein <amir73il@gmail.com>
Cc:     kbuild@lists.01.org,
        Gabriel Krisman Bertazi <krisman@collabora.com>,
        kbuild test robot <lkp@intel.com>, kbuild-all@lists.01.org,
        "Darrick J. Wong" <djwong@kernel.org>,
        Theodore Tso <tytso@mit.edu>,
        Dave Chinner <david@fromorbit.com>, Jan Kara <jack@suse.com>,
        David Howells <dhowells@redhat.com>,
        Khazhismel Kumykov <khazhy@google.com>,
        linux-fsdevel <linux-fsdevel@vger.kernel.org>,
        Ext4 <linux-ext4@vger.kernel.org>, kernel@collabora.com
Subject: Re: [PATCH v3 07/15] fsnotify: pass arguments of fsnotify() in
 struct fsnotify_event_info
Message-ID: <20210630104904.GS2040@kadam>
References: <20210629191035.681913-8-krisman@collabora.com>
 <202106300707.Xg0LaEwy-lkp@intel.com>
 <CAOQ4uxgRbpzo-AvvBxLQ5ARdFuX53RG+JpPOG8CDoEM2MdsWQQ@mail.gmail.com>
 <20210630084555.GH1983@kadam>
 <CAOQ4uxiCYBL2-FVMbn2RWcQnueueVoAd5sBtte+twLoU9eyFgA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAOQ4uxiCYBL2-FVMbn2RWcQnueueVoAd5sBtte+twLoU9eyFgA@mail.gmail.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

I think my bug report was not clear...  :/  The code looks like this:

	sb = inode->i_sb;

	if (inode) ...

The NULL check cannot be false because if "inode" is NULL we would have
already crashed when we dereference it on the line before.

In this case, based on last years discussion, the "inode" pointer can't
be NULL.  The debate is only whether the unnecessary NULL checks help
readability or hurt readability.

> Why does it presume that event_info->dir is non-NULL?

That was my commentary, just from reading the code.  Smatch says that
"event->dir" is unknown.

> Did smach check all the callers to fsnotify() or something?

The kbuild-bot doesn't build the cross function database but if you did
use the cross function database then, yes, it does track all the
callers.  There are two pointers that we care about, the "inode" and
the parent inode (dir).  Smatch can figure out when "inode" is NULL vs
non-NULL but where it gets stuck is on the some of the parent inodes
like this call from fsnotify_dirent():

	fsnotify_name(dir, mask, d_inode(dentry), &dentry->d_name, 0);
                                 ^^^^^^^^^^^^^^^

Smatch doesn't know that d_inode() is always non-NULL at this point.

regards,
dan carpenter