2020-08-06 06:07:51

by kernel test robot

[permalink] [raw]
Subject: fs/fuse/virtio_fs.c:1009:6: warning: Variable 'err' is reassigned a value before the old one has been used.

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 <[email protected]>


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;
^
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/[email protected]


2020-08-06 17:45:50

by Vivek Goyal

[permalink] [raw]
Subject: Re: fs/fuse/virtio_fs.c:1009:6: warning: Variable 'err' is reassigned a value before the old one has been used.

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 <[email protected]>
>
>
> 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/[email protected]
>

2020-08-10 09:27:58

by Xia, Hui

[permalink] [raw]
Subject: RE: [kbuild-all] Re: fs/fuse/virtio_fs.c:1009:6: warning: Variable 'err' is reassigned a value before the old one has been used.



>-----Original Message-----
>From: Vivek Goyal <[email protected]>
>Sent: 2020年8月7日 1:41
>To: kernel test robot <[email protected]>
>Cc: Stefan Hajnoczi <[email protected]>; [email protected]; linux-
>[email protected]; Miklos Szeredi <[email protected]>
>Subject: [kbuild-all] Re: fs/fuse/virtio_fs.c:1009:6: warning: Variable 'err' is
>reassigned a value before the old one has been used.
>
>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 <[email protected]>
>>
>>
>> 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. Please ignore this report.
We will double check this type of warning before send report in future. Sorry for inconvenient.

Thanks,
Hui

>
>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/[email protected]
>>
>_______________________________________________
>kbuild-all mailing list -- [email protected] To unsubscribe send an email to
>[email protected]