linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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.
Date: Thu, 6 Aug 2020 13:40:49 -0400	[thread overview]
Message-ID: <20200806174049.GE367847@redhat.com> (raw)
In-Reply-To: <202008061415.sHRFePnn%lkp@intel.com>

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
> 


  reply	other threads:[~2020-08-06 17:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-06  6:04 fs/fuse/virtio_fs.c:1009:6: warning: Variable 'err' is reassigned a value before the old one has been used kernel test robot
2020-08-06 17:40 ` Vivek Goyal [this message]
2020-08-10  9:24   ` [kbuild-all] " Xia, Hui

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200806174049.GE367847@redhat.com \
    --to=vgoyal@redhat.com \
    --cc=kbuild-all@lists.01.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkp@intel.com \
    --cc=mszeredi@redhat.com \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).