Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 6 Aug 2020 13:40:49 -0400
From:   Vivek Goyal <vgoyal@redhat.com>
To:     kernel test robot <lkp@intel.com>
Cc:     Stefan Hajnoczi <stefanha@redhat.com>, kbuild-all@lists.01.org,
        linux-kernel@vger.kernel.org, Miklos Szeredi <mszeredi@redhat.com>
Subject: Re: fs/fuse/virtio_fs.c:1009:6: warning: Variable 'err' is
 reassigned a value before the old one has been used.
Message-ID: <20200806174049.GE367847@redhat.com>
References: <202008061415.sHRFePnn%lkp@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <202008061415.sHRFePnn%lkp@intel.com>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Aug 06, 2020 at 02:04:18PM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   fffe3ae0ee84e25d2befe2ae59bc32aa2b6bc77b
> commit: a62a8ef9d97da23762a588592c8b8eb50a8deb6a virtio-fs: add virtiofs filesystem
> date:   11 months ago
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> cppcheck warnings: (new ones prefixed by >>)
> 
> >> fs/fuse/virtio_fs.c:1009:6: warning: Variable 'err' is reassigned a value before the old one has been used. [redundantAssignment]
>     err = -ENOMEM;
>         ^
>    fs/fuse/virtio_fs.c:1003:6: note: Variable 'err' is reassigned a value before the old one has been used.
>     err = -EINVAL;
>         ^
>    fs/fuse/virtio_fs.c:1009:6: note: Variable 'err' is reassigned a value before the old one has been used.
>     err = -ENOMEM;
>         ^
>    fs/fuse/virtio_fs.c:1020:6: warning: Variable 'err' is reassigned a value before the old one has been used. [redundantAssignment]
>     err = fuse_fill_super_common(sb, &ctx);
>         ^
>    fs/fuse/virtio_fs.c:1009:6: note: Variable 'err' is reassigned a value before the old one has been used.
>     err = -ENOMEM;

Not sure why this is a warning. Previous value is not going to be used
anymore. If it were to be used, we would have taken "goto err" path,
and return err. Very fact that we did not take that path means we
are not going to use previous value of err, hence it is ok to override
it. 

IOW, this seems like a false warning from compiler. This pattern has been
used in fuse and overlayfs code all over the place.

Thanks
Vivek

>         ^
>    fs/fuse/virtio_fs.c:1020:6: note: Variable 'err' is reassigned a value before the old one has been used.
>     err = fuse_fill_super_common(sb, &ctx);
>         ^
> 
> vim +/err +1009 fs/fuse/virtio_fs.c
> 
>    979	
>    980	static int virtio_fs_fill_super(struct super_block *sb)
>    981	{
>    982		struct fuse_conn *fc = get_fuse_conn_super(sb);
>    983		struct virtio_fs *fs = fc->iq.priv;
>    984		unsigned int i;
>    985		int err;
>    986		struct fuse_fs_context ctx = {
>    987			.rootmode = S_IFDIR,
>    988			.default_permissions = 1,
>    989			.allow_other = 1,
>    990			.max_read = UINT_MAX,
>    991			.blksize = 512,
>    992			.destroy = true,
>    993			.no_control = true,
>    994			.no_force_umount = true,
>    995		};
>    996	
>    997		mutex_lock(&virtio_fs_mutex);
>    998	
>    999		/* After holding mutex, make sure virtiofs device is still there.
>   1000		 * Though we are holding a reference to it, drive ->remove might
>   1001		 * still have cleaned up virtual queues. In that case bail out.
>   1002		 */
>   1003		err = -EINVAL;
>   1004		if (list_empty(&fs->list)) {
>   1005			pr_info("virtio-fs: tag <%s> not found\n", fs->tag);
>   1006			goto err;
>   1007		}
>   1008	
> > 1009		err = -ENOMEM;
>   1010		/* Allocate fuse_dev for hiprio and notification queues */
>   1011		for (i = 0; i < VQ_REQUEST; i++) {
>   1012			struct virtio_fs_vq *fsvq = &fs->vqs[i];
>   1013	
>   1014			fsvq->fud = fuse_dev_alloc();
>   1015			if (!fsvq->fud)
>   1016				goto err_free_fuse_devs;
>   1017		}
>   1018	
>   1019		ctx.fudptr = (void **)&fs->vqs[VQ_REQUEST].fud;
>   1020		err = fuse_fill_super_common(sb, &ctx);
>   1021		if (err < 0)
>   1022			goto err_free_fuse_devs;
>   1023	
>   1024		fc = fs->vqs[VQ_REQUEST].fud->fc;
>   1025	
>   1026		for (i = 0; i < fs->nvqs; i++) {
>   1027			struct virtio_fs_vq *fsvq = &fs->vqs[i];
>   1028	
>   1029			if (i == VQ_REQUEST)
>   1030				continue; /* already initialized */
>   1031			fuse_dev_install(fsvq->fud, fc);
>   1032		}
>   1033	
>   1034		/* Previous unmount will stop all queues. Start these again */
>   1035		virtio_fs_start_all_queues(fs);
>   1036		fuse_send_init(fc);
>   1037		mutex_unlock(&virtio_fs_mutex);
>   1038		return 0;
>   1039	
>   1040	err_free_fuse_devs:
>   1041		virtio_fs_free_devs(fs);
>   1042	err:
>   1043		mutex_unlock(&virtio_fs_mutex);
>   1044		return err;
>   1045	}
>   1046	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>