All of lore.kernel.org
 help / color / mirror / Atom feed
* [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
@ 2022-07-01  8:49 kernel test robot
  2022-07-01  8:59   ` Greg Kroah-Hartman
  0 siblings, 1 reply; 15+ messages in thread
From: kernel test robot @ 2022-07-01  8:49 UTC (permalink / raw)
  To: Shuah Khan; +Cc: llvm, kbuild-all, linux-kernel, Greg Kroah-Hartman

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
head:   3776c78559853fd151be7c41e369fd076fb679d5
commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
        git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
        git fetch --no-tags char-misc char-misc-linus
        git checkout 3776c78559853fd151be7c41e369fd076fb679d5
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!ucr->rsp_buf)
               ^~~~~~~~~~~~~
   drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
           return ret;
                  ^~~
   drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
           if (!ucr->rsp_buf)
           ^~~~~~~~~~~~~~~~~~
   drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
           int ret;
                  ^
                   = 0
   1 warning generated.


vim +639 drivers/misc/cardreader/rtsx_usb.c

   616	
   617	static int rtsx_usb_probe(struct usb_interface *intf,
   618				 const struct usb_device_id *id)
   619	{
   620		struct usb_device *usb_dev = interface_to_usbdev(intf);
   621		struct rtsx_ucr *ucr;
   622		int ret;
   623	
   624		dev_dbg(&intf->dev,
   625			": Realtek USB Card Reader found at bus %03d address %03d\n",
   626			 usb_dev->bus->busnum, usb_dev->devnum);
   627	
   628		ucr = devm_kzalloc(&intf->dev, sizeof(*ucr), GFP_KERNEL);
   629		if (!ucr)
   630			return -ENOMEM;
   631	
   632		ucr->pusb_dev = usb_dev;
   633	
   634		ucr->cmd_buf = kmalloc(IOBUF_SIZE, GFP_KERNEL);
   635		if (!ucr->cmd_buf)
   636			return -ENOMEM;
   637	
   638		ucr->rsp_buf = kmalloc(IOBUF_SIZE, GFP_KERNEL);
 > 639		if (!ucr->rsp_buf)
   640			goto out_free_cmd_buf;
   641	
   642		usb_set_intfdata(intf, ucr);
   643	
   644		ucr->vendor_id = id->idVendor;
   645		ucr->product_id = id->idProduct;
   646	
   647		mutex_init(&ucr->dev_mutex);
   648	
   649		ucr->pusb_intf = intf;
   650	
   651		/* initialize */
   652		ret = rtsx_usb_init_chip(ucr);
   653		if (ret)
   654			goto out_init_fail;
   655	
   656		/* initialize USB SG transfer timer */
   657		timer_setup(&ucr->sg_timer, rtsx_usb_sg_timed_out, 0);
   658	
   659		ret = mfd_add_hotplug_devices(&intf->dev, rtsx_usb_cells,
   660					      ARRAY_SIZE(rtsx_usb_cells));
   661		if (ret)
   662			goto out_init_fail;
   663	

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
  2022-07-01  8:49 [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true kernel test robot
@ 2022-07-01  8:59   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2022-07-01  8:59 UTC (permalink / raw)
  To: kernel test robot; +Cc: Shuah Khan, llvm, kbuild-all, linux-kernel

On Fri, Jul 01, 2022 at 04:49:50PM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
> head:   3776c78559853fd151be7c41e369fd076fb679d5
> commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
> config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp@intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install arm cross compiling tool for clang build
>         # apt-get install binutils-arm-linux-gnueabi
>         # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
>         git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
>         git fetch --no-tags char-misc char-misc-linus
>         git checkout 3776c78559853fd151be7c41e369fd076fb679d5
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!ucr->rsp_buf)
>                ^~~~~~~~~~~~~
>    drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>            return ret;
>                   ^~~
>    drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>            if (!ucr->rsp_buf)
>            ^~~~~~~~~~~~~~~~~~
>    drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>            int ret;
>                   ^
>                    = 0
>    1 warning generated.

Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
patch to fix this?  The warning does look correct.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
@ 2022-07-01  8:59   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 15+ messages in thread
From: Greg Kroah-Hartman @ 2022-07-01  8:59 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2527 bytes --]

On Fri, Jul 01, 2022 at 04:49:50PM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
> head:   3776c78559853fd151be7c41e369fd076fb679d5
> commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
> config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp(a)intel.com/config)
> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install arm cross compiling tool for clang build
>         # apt-get install binutils-arm-linux-gnueabi
>         # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
>         git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
>         git fetch --no-tags char-misc char-misc-linus
>         git checkout 3776c78559853fd151be7c41e369fd076fb679d5
>         # save the config file
>         mkdir build_dir && cp config build_dir/.config
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/
> 
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
> 
> All warnings (new ones prefixed by >>):
> 
> >> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>            if (!ucr->rsp_buf)
>                ^~~~~~~~~~~~~
>    drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>            return ret;
>                   ^~~
>    drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>            if (!ucr->rsp_buf)
>            ^~~~~~~~~~~~~~~~~~
>    drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>            int ret;
>                   ^
>                    = 0
>    1 warning generated.

Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
patch to fix this?  The warning does look correct.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
  2022-07-01  8:59   ` Greg Kroah-Hartman
@ 2022-07-01 14:48     ` Shuah Khan
  -1 siblings, 0 replies; 15+ messages in thread
From: Shuah Khan @ 2022-07-01 14:48 UTC (permalink / raw)
  To: Greg Kroah-Hartman, kernel test robot
  Cc: llvm, kbuild-all, linux-kernel, Shuah Khan

On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
> On Fri, Jul 01, 2022 at 04:49:50PM +0800, kernel test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
>> head:   3776c78559853fd151be7c41e369fd076fb679d5
>> commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
>> config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp@intel.com/config)
>> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
>> reproduce (this is a W=1 build):
>>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # install arm cross compiling tool for clang build
>>          # apt-get install binutils-arm-linux-gnueabi
>>          # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
>>          git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
>>          git fetch --no-tags char-misc char-misc-linus
>>          git checkout 3776c78559853fd151be7c41e369fd076fb679d5
>>          # save the config file
>>          mkdir build_dir && cp config build_dir/.config
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/
>>
>> If you fix the issue, kindly add following tag where applicable
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> All warnings (new ones prefixed by >>):
>>
>>>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!ucr->rsp_buf)
>>                 ^~~~~~~~~~~~~
>>     drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>>             return ret;
>>                    ^~~
>>     drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>>             if (!ucr->rsp_buf)
>>             ^~~~~~~~~~~~~~~~~~
>>     drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>>             int ret;
>>                    ^
>>                     = 0
>>     1 warning generated.
> 
> Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
> patch to fix this?  The warning does look correct.
> 

gcc didn't complain when I compiled either. I will send a follow-on patch.

thanks,
-- Shuah


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
@ 2022-07-01 14:48     ` Shuah Khan
  0 siblings, 0 replies; 15+ messages in thread
From: Shuah Khan @ 2022-07-01 14:48 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2720 bytes --]

On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
> On Fri, Jul 01, 2022 at 04:49:50PM +0800, kernel test robot wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
>> head:   3776c78559853fd151be7c41e369fd076fb679d5
>> commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
>> config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp(a)intel.com/config)
>> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
>> reproduce (this is a W=1 build):
>>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>          chmod +x ~/bin/make.cross
>>          # install arm cross compiling tool for clang build
>>          # apt-get install binutils-arm-linux-gnueabi
>>          # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
>>          git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
>>          git fetch --no-tags char-misc char-misc-linus
>>          git checkout 3776c78559853fd151be7c41e369fd076fb679d5
>>          # save the config file
>>          mkdir build_dir && cp config build_dir/.config
>>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/
>>
>> If you fix the issue, kindly add following tag where applicable
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> All warnings (new ones prefixed by >>):
>>
>>>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>             if (!ucr->rsp_buf)
>>                 ^~~~~~~~~~~~~
>>     drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>>             return ret;
>>                    ^~~
>>     drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>>             if (!ucr->rsp_buf)
>>             ^~~~~~~~~~~~~~~~~~
>>     drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>>             int ret;
>>                    ^
>>                     = 0
>>     1 warning generated.
> 
> Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
> patch to fix this?  The warning does look correct.
> 

gcc didn't complain when I compiled either. I will send a follow-on patch.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
  2022-07-01 14:48     ` Shuah Khan
@ 2022-07-01 15:39       ` Nathan Chancellor
  -1 siblings, 0 replies; 15+ messages in thread
From: Nathan Chancellor @ 2022-07-01 15:39 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Greg Kroah-Hartman, kernel test robot, llvm, kbuild-all, linux-kernel

On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
> On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
> > On Fri, Jul 01, 2022 at 04:49:50PM +0800, kernel test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
> > > head:   3776c78559853fd151be7c41e369fd076fb679d5
> > > commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
> > > config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp@intel.com/config)
> > > compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
> > > reproduce (this is a W=1 build):
> > >          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >          chmod +x ~/bin/make.cross
> > >          # install arm cross compiling tool for clang build
> > >          # apt-get install binutils-arm-linux-gnueabi
> > >          # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
> > >          git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> > >          git fetch --no-tags char-misc char-misc-linus
> > >          git checkout 3776c78559853fd151be7c41e369fd076fb679d5
> > >          # save the config file
> > >          mkdir build_dir && cp config build_dir/.config
> > >          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/
> > > 
> > > If you fix the issue, kindly add following tag where applicable
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > > > > drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!ucr->rsp_buf)
> > >                 ^~~~~~~~~~~~~
> > >     drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
> > >             return ret;
> > >                    ^~~
> > >     drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
> > >             if (!ucr->rsp_buf)
> > >             ^~~~~~~~~~~~~~~~~~
> > >     drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
> > >             int ret;
> > >                    ^
> > >                     = 0
> > >     1 warning generated.
> > 
> > Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
> > patch to fix this?  The warning does look correct.
> > 
> 
> gcc didn't complain when I compiled either. I will send a follow-on patch.

Unfortunately, GCC won't warn for most uninitialized variables by
default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
games with -Wno-maybe-initialized"). They will potentially show up at
W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
case):

| drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
| drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
|   678 |         return ret;
|       |                ^~~
| drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
|   622 |         int ret;
|       |             ^~~
| cc1: all warnings being treated as errors

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
@ 2022-07-01 15:39       ` Nathan Chancellor
  0 siblings, 0 replies; 15+ messages in thread
From: Nathan Chancellor @ 2022-07-01 15:39 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3637 bytes --]

On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
> On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
> > On Fri, Jul 01, 2022 at 04:49:50PM +0800, kernel test robot wrote:
> > > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
> > > head:   3776c78559853fd151be7c41e369fd076fb679d5
> > > commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
> > > config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp(a)intel.com/config)
> > > compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
> > > reproduce (this is a W=1 build):
> > >          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > >          chmod +x ~/bin/make.cross
> > >          # install arm cross compiling tool for clang build
> > >          # apt-get install binutils-arm-linux-gnueabi
> > >          # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
> > >          git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
> > >          git fetch --no-tags char-misc char-misc-linus
> > >          git checkout 3776c78559853fd151be7c41e369fd076fb679d5
> > >          # save the config file
> > >          mkdir build_dir && cp config build_dir/.config
> > >          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/
> > > 
> > > If you fix the issue, kindly add following tag where applicable
> > > Reported-by: kernel test robot <lkp@intel.com>
> > > 
> > > All warnings (new ones prefixed by >>):
> > > 
> > > > > drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > >             if (!ucr->rsp_buf)
> > >                 ^~~~~~~~~~~~~
> > >     drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
> > >             return ret;
> > >                    ^~~
> > >     drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
> > >             if (!ucr->rsp_buf)
> > >             ^~~~~~~~~~~~~~~~~~
> > >     drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
> > >             int ret;
> > >                    ^
> > >                     = 0
> > >     1 warning generated.
> > 
> > Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
> > patch to fix this?  The warning does look correct.
> > 
> 
> gcc didn't complain when I compiled either. I will send a follow-on patch.

Unfortunately, GCC won't warn for most uninitialized variables by
default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
games with -Wno-maybe-initialized"). They will potentially show up at
W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
case):

| drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
| drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
|   678 |         return ret;
|       |                ^~~
| drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
|   622 |         int ret;
|       |             ^~~
| cc1: all warnings being treated as errors

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
  2022-07-01 15:39       ` Nathan Chancellor
@ 2022-07-01 15:52         ` Shuah Khan
  -1 siblings, 0 replies; 15+ messages in thread
From: Shuah Khan @ 2022-07-01 15:52 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Greg Kroah-Hartman, kernel test robot, llvm, kbuild-all,
	linux-kernel, Shuah Khan

On 7/1/22 9:39 AM, Nathan Chancellor wrote:
> On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
>> On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
>>> On Fri, Jul 01, 2022 at 04:49:50PM +0800, kernel test robot wrote:
>>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
>>>> head:   3776c78559853fd151be7c41e369fd076fb679d5
>>>> commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
>>>> config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp@intel.com/config)
>>>> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
>>>> reproduce (this is a W=1 build):
>>>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>>           chmod +x ~/bin/make.cross
>>>>           # install arm cross compiling tool for clang build
>>>>           # apt-get install binutils-arm-linux-gnueabi
>>>>           # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
>>>>           git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
>>>>           git fetch --no-tags char-misc char-misc-linus
>>>>           git checkout 3776c78559853fd151be7c41e369fd076fb679d5
>>>>           # save the config file
>>>>           mkdir build_dir && cp config build_dir/.config
>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/
>>>>
>>>> If you fix the issue, kindly add following tag where applicable
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>>>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>>>              if (!ucr->rsp_buf)
>>>>                  ^~~~~~~~~~~~~
>>>>      drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>>>>              return ret;
>>>>                     ^~~
>>>>      drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>>>>              if (!ucr->rsp_buf)
>>>>              ^~~~~~~~~~~~~~~~~~
>>>>      drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>>>>              int ret;
>>>>                     ^
>>>>                      = 0
>>>>      1 warning generated.
>>>
>>> Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
>>> patch to fix this?  The warning does look correct.
>>>
>>
>> gcc didn't complain when I compiled either. I will send a follow-on patch.
> 
> Unfortunately, GCC won't warn for most uninitialized variables by
> default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
> games with -Wno-maybe-initialized"). They will potentially show up at
> W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
> case):
> 

Thank you.

> | drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
> | drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
> |   678 |         return ret;
> |       |                ^~~
> | drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
> |   622 |         int ret;
> |       |             ^~~
> | cc1: all warnings being treated as errors
> 

This is a bug and a good find. ret should have been set
in the if (!ucr->rsp_buf) before going to error handling.

I wonder if it would have been flagged if ret were to be
initialized to 0. Something to experiment.

thanks,
-- Shuah


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
@ 2022-07-01 15:52         ` Shuah Khan
  0 siblings, 0 replies; 15+ messages in thread
From: Shuah Khan @ 2022-07-01 15:52 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3942 bytes --]

On 7/1/22 9:39 AM, Nathan Chancellor wrote:
> On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
>> On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
>>> On Fri, Jul 01, 2022 at 04:49:50PM +0800, kernel test robot wrote:
>>>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git char-misc-linus
>>>> head:   3776c78559853fd151be7c41e369fd076fb679d5
>>>> commit: 3776c78559853fd151be7c41e369fd076fb679d5 [3/3] misc: rtsx_usb: use separate command and response buffers
>>>> config: arm-buildonly-randconfig-r006-20220629 (https://download.01.org/0day-ci/archive/20220701/202207011658.rHJFVLWA-lkp(a)intel.com/config)
>>>> compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project a9119143a2d1f4d0d0bc1fe0d819e5351b4e0deb)
>>>> reproduce (this is a W=1 build):
>>>>           wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>>>           chmod +x ~/bin/make.cross
>>>>           # install arm cross compiling tool for clang build
>>>>           # apt-get install binutils-arm-linux-gnueabi
>>>>           # https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git/commit/?id=3776c78559853fd151be7c41e369fd076fb679d5
>>>>           git remote add char-misc https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
>>>>           git fetch --no-tags char-misc char-misc-linus
>>>>           git checkout 3776c78559853fd151be7c41e369fd076fb679d5
>>>>           # save the config file
>>>>           mkdir build_dir && cp config build_dir/.config
>>>>           COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/misc/cardreader/
>>>>
>>>> If you fix the issue, kindly add following tag where applicable
>>>> Reported-by: kernel test robot <lkp@intel.com>
>>>>
>>>> All warnings (new ones prefixed by >>):
>>>>
>>>>>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>>>              if (!ucr->rsp_buf)
>>>>                  ^~~~~~~~~~~~~
>>>>      drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>>>>              return ret;
>>>>                     ^~~
>>>>      drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>>>>              if (!ucr->rsp_buf)
>>>>              ^~~~~~~~~~~~~~~~~~
>>>>      drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>>>>              int ret;
>>>>                     ^
>>>>                      = 0
>>>>      1 warning generated.
>>>
>>> Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
>>> patch to fix this?  The warning does look correct.
>>>
>>
>> gcc didn't complain when I compiled either. I will send a follow-on patch.
> 
> Unfortunately, GCC won't warn for most uninitialized variables by
> default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
> games with -Wno-maybe-initialized"). They will potentially show up at
> W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
> case):
> 

Thank you.

> | drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
> | drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
> |   678 |         return ret;
> |       |                ^~~
> | drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
> |   622 |         int ret;
> |       |             ^~~
> | cc1: all warnings being treated as errors
> 

This is a bug and a good find. ret should have been set
in the if (!ucr->rsp_buf) before going to error handling.

I wonder if it would have been flagged if ret were to be
initialized to 0. Something to experiment.

thanks,
-- Shuah

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
  2022-07-01 15:52         ` Shuah Khan
@ 2022-07-01 16:09           ` Shuah Khan
  -1 siblings, 0 replies; 15+ messages in thread
From: Shuah Khan @ 2022-07-01 16:09 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Greg Kroah-Hartman, kernel test robot, llvm, kbuild-all,
	linux-kernel, Shuah Khan

On 7/1/22 9:52 AM, Shuah Khan wrote:
> On 7/1/22 9:39 AM, Nathan Chancellor wrote:
>> On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
>>> On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:

>>>>> All warnings (new ones prefixed by >>):
>>>>>
>>>>>>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>>>>              if (!ucr->rsp_buf)
>>>>>                  ^~~~~~~~~~~~~
>>>>>      drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>>>>>              return ret;
>>>>>                     ^~~
>>>>>      drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>>>>>              if (!ucr->rsp_buf)
>>>>>              ^~~~~~~~~~~~~~~~~~
>>>>>      drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>>>>>              int ret;
>>>>>                     ^
>>>>>                      = 0
>>>>>      1 warning generated.
>>>>
>>>> Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
>>>> patch to fix this?  The warning does look correct.
>>>>
>>>
>>> gcc didn't complain when I compiled either. I will send a follow-on patch.
>>
>> Unfortunately, GCC won't warn for most uninitialized variables by
>> default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
>> games with -Wno-maybe-initialized"). They will potentially show up at
>> W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
>> case):
>>
> 
> Thank you.
> 
>> | drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
>> | drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
>> |   678 |         return ret;
>> |       |                ^~~
>> | drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
>> |   622 |         int ret;
>> |       |             ^~~
>> | cc1: all warnings being treated as errors
>>
> 
> This is a bug and a good find. ret should have been set
> in the if (!ucr->rsp_buf) before going to error handling.
> 
> I wonder if it would have been flagged if ret were to be
> initialized to 0. Something to experiment.
> 

I had to try. As I suspected initializing ret will mask this bug.

KCFLAGS=-Wmaybe-uninitialized will not flag it even though
the bug still exists. It will return 0 when memory allocation
fails.

Initializing isn't always the right answer for these kinds of
warnings.

thanks,
-- Shuah



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
@ 2022-07-01 16:09           ` Shuah Khan
  0 siblings, 0 replies; 15+ messages in thread
From: Shuah Khan @ 2022-07-01 16:09 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2841 bytes --]

On 7/1/22 9:52 AM, Shuah Khan wrote:
> On 7/1/22 9:39 AM, Nathan Chancellor wrote:
>> On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
>>> On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:

>>>>> All warnings (new ones prefixed by >>):
>>>>>
>>>>>>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>>>>              if (!ucr->rsp_buf)
>>>>>                  ^~~~~~~~~~~~~
>>>>>      drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>>>>>              return ret;
>>>>>                     ^~~
>>>>>      drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>>>>>              if (!ucr->rsp_buf)
>>>>>              ^~~~~~~~~~~~~~~~~~
>>>>>      drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>>>>>              int ret;
>>>>>                     ^
>>>>>                      = 0
>>>>>      1 warning generated.
>>>>
>>>> Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
>>>> patch to fix this?  The warning does look correct.
>>>>
>>>
>>> gcc didn't complain when I compiled either. I will send a follow-on patch.
>>
>> Unfortunately, GCC won't warn for most uninitialized variables by
>> default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
>> games with -Wno-maybe-initialized"). They will potentially show up at
>> W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
>> case):
>>
> 
> Thank you.
> 
>> | drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
>> | drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
>> |   678 |         return ret;
>> |       |                ^~~
>> | drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
>> |   622 |         int ret;
>> |       |             ^~~
>> | cc1: all warnings being treated as errors
>>
> 
> This is a bug and a good find. ret should have been set
> in the if (!ucr->rsp_buf) before going to error handling.
> 
> I wonder if it would have been flagged if ret were to be
> initialized to 0. Something to experiment.
> 

I had to try. As I suspected initializing ret will mask this bug.

KCFLAGS=-Wmaybe-uninitialized will not flag it even though
the bug still exists. It will return 0 when memory allocation
fails.

Initializing isn't always the right answer for these kinds of
warnings.

thanks,
-- Shuah


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
  2022-07-01 16:09           ` Shuah Khan
@ 2022-07-01 16:27             ` Nathan Chancellor
  -1 siblings, 0 replies; 15+ messages in thread
From: Nathan Chancellor @ 2022-07-01 16:27 UTC (permalink / raw)
  To: Shuah Khan
  Cc: Greg Kroah-Hartman, kernel test robot, llvm, kbuild-all, linux-kernel

On Fri, Jul 01, 2022 at 10:09:45AM -0600, Shuah Khan wrote:
> On 7/1/22 9:52 AM, Shuah Khan wrote:
> > On 7/1/22 9:39 AM, Nathan Chancellor wrote:
> > > On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
> > > > On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
> 
> > > > > > All warnings (new ones prefixed by >>):
> > > > > > 
> > > > > > > > drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > > > > >              if (!ucr->rsp_buf)
> > > > > >                  ^~~~~~~~~~~~~
> > > > > >      drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
> > > > > >              return ret;
> > > > > >                     ^~~
> > > > > >      drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
> > > > > >              if (!ucr->rsp_buf)
> > > > > >              ^~~~~~~~~~~~~~~~~~
> > > > > >      drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
> > > > > >              int ret;
> > > > > >                     ^
> > > > > >                      = 0
> > > > > >      1 warning generated.
> > > > > 
> > > > > Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
> > > > > patch to fix this?  The warning does look correct.
> > > > > 
> > > > 
> > > > gcc didn't complain when I compiled either. I will send a follow-on patch.
> > > 
> > > Unfortunately, GCC won't warn for most uninitialized variables by
> > > default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
> > > games with -Wno-maybe-initialized"). They will potentially show up at
> > > W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
> > > case):
> > > 
> > 
> > Thank you.
> > 
> > > | drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
> > > | drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
> > > |   678 |         return ret;
> > > |       |                ^~~
> > > | drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
> > > |   622 |         int ret;
> > > |       |             ^~~
> > > | cc1: all warnings being treated as errors
> > > 
> > 
> > This is a bug and a good find. ret should have been set
> > in the if (!ucr->rsp_buf) before going to error handling.
> > 
> > I wonder if it would have been flagged if ret were to be
> > initialized to 0. Something to experiment.
> > 
> 
> I had to try. As I suspected initializing ret will mask this bug.
> 
> KCFLAGS=-Wmaybe-uninitialized will not flag it even though
> the bug still exists. It will return 0 w hen memory allocation
> fails.

Right, if the variable is unconditonally initialized at the top of the
function, it can never be flagged as uninitialized but that doesn't mean
the value is correct for every branch within the function.

> Initializing isn't always the right answer for these kinds of
> warnings.

I would say "unconditional initialization", as ret has to be initialized
somewhere to fix this warning, right? If you are referring to clang's
'ret = 0' suggestion, I agree that it can be misleading (especially in
the case of suggesting initializing dereferenced pointers to NULL...,
see [1]) but at the end of the day, it is still on the programmer to
analyze their code fully in light of the warning before implementing a
fix. Clang cannot know that the return code should be set in the if
statement rather than at the top of the function.

[1]: https://github.com/llvm/llvm-project/issues/51901

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
@ 2022-07-01 16:27             ` Nathan Chancellor
  0 siblings, 0 replies; 15+ messages in thread
From: Nathan Chancellor @ 2022-07-01 16:27 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3945 bytes --]

On Fri, Jul 01, 2022 at 10:09:45AM -0600, Shuah Khan wrote:
> On 7/1/22 9:52 AM, Shuah Khan wrote:
> > On 7/1/22 9:39 AM, Nathan Chancellor wrote:
> > > On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
> > > > On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
> 
> > > > > > All warnings (new ones prefixed by >>):
> > > > > > 
> > > > > > > > drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
> > > > > >              if (!ucr->rsp_buf)
> > > > > >                  ^~~~~~~~~~~~~
> > > > > >      drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
> > > > > >              return ret;
> > > > > >                     ^~~
> > > > > >      drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
> > > > > >              if (!ucr->rsp_buf)
> > > > > >              ^~~~~~~~~~~~~~~~~~
> > > > > >      drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
> > > > > >              int ret;
> > > > > >                     ^
> > > > > >                      = 0
> > > > > >      1 warning generated.
> > > > > 
> > > > > Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
> > > > > patch to fix this?  The warning does look correct.
> > > > > 
> > > > 
> > > > gcc didn't complain when I compiled either. I will send a follow-on patch.
> > > 
> > > Unfortunately, GCC won't warn for most uninitialized variables by
> > > default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
> > > games with -Wno-maybe-initialized"). They will potentially show up at
> > > W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
> > > case):
> > > 
> > 
> > Thank you.
> > 
> > > | drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
> > > | drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
> > > |   678 |         return ret;
> > > |       |                ^~~
> > > | drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
> > > |   622 |         int ret;
> > > |       |             ^~~
> > > | cc1: all warnings being treated as errors
> > > 
> > 
> > This is a bug and a good find. ret should have been set
> > in the if (!ucr->rsp_buf) before going to error handling.
> > 
> > I wonder if it would have been flagged if ret were to be
> > initialized to 0. Something to experiment.
> > 
> 
> I had to try. As I suspected initializing ret will mask this bug.
> 
> KCFLAGS=-Wmaybe-uninitialized will not flag it even though
> the bug still exists. It will return 0 w hen memory allocation
> fails.

Right, if the variable is unconditonally initialized at the top of the
function, it can never be flagged as uninitialized but that doesn't mean
the value is correct for every branch within the function.

> Initializing isn't always the right answer for these kinds of
> warnings.

I would say "unconditional initialization", as ret has to be initialized
somewhere to fix this warning, right? If you are referring to clang's
'ret = 0' suggestion, I agree that it can be misleading (especially in
the case of suggesting initializing dereferenced pointers to NULL...,
see [1]) but at the end of the day, it is still on the programmer to
analyze their code fully in light of the warning before implementing a
fix. Clang cannot know that the return code should be set in the if
statement rather than at the top of the function.

[1]: https://github.com/llvm/llvm-project/issues/51901

Cheers,
Nathan

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
  2022-07-01 16:27             ` Nathan Chancellor
@ 2022-07-01 16:59               ` Shuah Khan
  -1 siblings, 0 replies; 15+ messages in thread
From: Shuah Khan @ 2022-07-01 16:59 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Greg Kroah-Hartman, kernel test robot, llvm, kbuild-all,
	linux-kernel, Shuah Khan

On 7/1/22 10:27 AM, Nathan Chancellor wrote:
> On Fri, Jul 01, 2022 at 10:09:45AM -0600, Shuah Khan wrote:
>> On 7/1/22 9:52 AM, Shuah Khan wrote:
>>> On 7/1/22 9:39 AM, Nathan Chancellor wrote:
>>>> On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
>>>>> On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
>>
>>>>>>> All warnings (new ones prefixed by >>):
>>>>>>>
>>>>>>>>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>>>>>>               if (!ucr->rsp_buf)
>>>>>>>                   ^~~~~~~~~~~~~
>>>>>>>       drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>>>>>>>               return ret;
>>>>>>>                      ^~~
>>>>>>>       drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>>>>>>>               if (!ucr->rsp_buf)
>>>>>>>               ^~~~~~~~~~~~~~~~~~
>>>>>>>       drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>>>>>>>               int ret;
>>>>>>>                      ^
>>>>>>>                       = 0
>>>>>>>       1 warning generated.
>>>>>>
>>>>>> Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
>>>>>> patch to fix this?  The warning does look correct.
>>>>>>
>>>>>
>>>>> gcc didn't complain when I compiled either. I will send a follow-on patch.
>>>>
>>>> Unfortunately, GCC won't warn for most uninitialized variables by
>>>> default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
>>>> games with -Wno-maybe-initialized"). They will potentially show up at
>>>> W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
>>>> case):
>>>>
>>>
>>> Thank you.
>>>
>>>> | drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
>>>> | drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
>>>> |   678 |         return ret;
>>>> |       |                ^~~
>>>> | drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
>>>> |   622 |         int ret;
>>>> |       |             ^~~
>>>> | cc1: all warnings being treated as errors
>>>>
>>>
>>> This is a bug and a good find. ret should have been set
>>> in the if (!ucr->rsp_buf) before going to error handling.
>>>
>>> I wonder if it would have been flagged if ret were to be
>>> initialized to 0. Something to experiment.
>>>
>>
>> I had to try. As I suspected initializing ret will mask this bug.
>>
>> KCFLAGS=-Wmaybe-uninitialized will not flag it even though
>> the bug still exists. It will return 0 w hen memory allocation
>> fails.
> 
> Right, if the variable is unconditonally initialized at the top of the
> function, it can never be flagged as uninitialized but that doesn't mean
> the value is correct for every branch within the function.
> 

+1

>> Initializing isn't always the right answer for these kinds of
>> warnings.
> 
> I would say "unconditional initialization", as ret has to be initialized
> somewhere to fix this warning, right? If you are referring to clang's
> 'ret = 0' suggestion, I agree that it can be misleading (especially in
> the case of suggesting initializing dereferenced pointers to NULL...,
> see [1]) but at the end of the day, it is still on the programmer to
> analyze their code fully in light of the warning before implementing a
> fix. Clang cannot know that the return code should be set in the if
> statement rather than at the top of the function.
> 

+1 on "programmer to analyze their code fully in light of the warning"

I am not necessarily referring to the clang warning. I agree that it could
be misleading and suggest or send us towards a wrong fix. It is on us to
be careful - this would be when fixing the problem and reviewing the fix.

thanks,
-- Shuah



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true
@ 2022-07-01 16:59               ` Shuah Khan
  0 siblings, 0 replies; 15+ messages in thread
From: Shuah Khan @ 2022-07-01 16:59 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 4189 bytes --]

On 7/1/22 10:27 AM, Nathan Chancellor wrote:
> On Fri, Jul 01, 2022 at 10:09:45AM -0600, Shuah Khan wrote:
>> On 7/1/22 9:52 AM, Shuah Khan wrote:
>>> On 7/1/22 9:39 AM, Nathan Chancellor wrote:
>>>> On Fri, Jul 01, 2022 at 08:48:11AM -0600, Shuah Khan wrote:
>>>>> On 7/1/22 2:59 AM, Greg Kroah-Hartman wrote:
>>
>>>>>>> All warnings (new ones prefixed by >>):
>>>>>>>
>>>>>>>>> drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
>>>>>>>               if (!ucr->rsp_buf)
>>>>>>>                   ^~~~~~~~~~~~~
>>>>>>>       drivers/misc/cardreader/rtsx_usb.c:678:9: note: uninitialized use occurs here
>>>>>>>               return ret;
>>>>>>>                      ^~~
>>>>>>>       drivers/misc/cardreader/rtsx_usb.c:639:2: note: remove the 'if' if its condition is always false
>>>>>>>               if (!ucr->rsp_buf)
>>>>>>>               ^~~~~~~~~~~~~~~~~~
>>>>>>>       drivers/misc/cardreader/rtsx_usb.c:622:9: note: initialize the variable 'ret' to silence this warning
>>>>>>>               int ret;
>>>>>>>                      ^
>>>>>>>                       = 0
>>>>>>>       1 warning generated.
>>>>>>
>>>>>> Odd, gcc doesn't show this for me.  Shuah, can you send a follow-on
>>>>>> patch to fix this?  The warning does look correct.
>>>>>>
>>>>>
>>>>> gcc didn't complain when I compiled either. I will send a follow-on patch.
>>>>
>>>> Unfortunately, GCC won't warn for most uninitialized variables by
>>>> default after 5.7, which included commit 78a5255ffb6a ("Stop the ad-hoc
>>>> games with -Wno-maybe-initialized"). They will potentially show up at
>>>> W=2 or with an explicit KCFLAGS=-Wmaybe-uninitialized (it does in this
>>>> case):
>>>>
>>>
>>> Thank you.
>>>
>>>> | drivers/misc/cardreader/rtsx_usb.c: In function ‘rtsx_usb_probe’:
>>>> | drivers/misc/cardreader/rtsx_usb.c:678:16: error: ‘ret’ may be used uninitialized [-Werror=maybe-uninitialized]
>>>> |   678 |         return ret;
>>>> |       |                ^~~
>>>> | drivers/misc/cardreader/rtsx_usb.c:622:13: note: ‘ret’ was declared here
>>>> |   622 |         int ret;
>>>> |       |             ^~~
>>>> | cc1: all warnings being treated as errors
>>>>
>>>
>>> This is a bug and a good find. ret should have been set
>>> in the if (!ucr->rsp_buf) before going to error handling.
>>>
>>> I wonder if it would have been flagged if ret were to be
>>> initialized to 0. Something to experiment.
>>>
>>
>> I had to try. As I suspected initializing ret will mask this bug.
>>
>> KCFLAGS=-Wmaybe-uninitialized will not flag it even though
>> the bug still exists. It will return 0 w hen memory allocation
>> fails.
> 
> Right, if the variable is unconditonally initialized at the top of the
> function, it can never be flagged as uninitialized but that doesn't mean
> the value is correct for every branch within the function.
> 

+1

>> Initializing isn't always the right answer for these kinds of
>> warnings.
> 
> I would say "unconditional initialization", as ret has to be initialized
> somewhere to fix this warning, right? If you are referring to clang's
> 'ret = 0' suggestion, I agree that it can be misleading (especially in
> the case of suggesting initializing dereferenced pointers to NULL...,
> see [1]) but at the end of the day, it is still on the programmer to
> analyze their code fully in light of the warning before implementing a
> fix. Clang cannot know that the return code should be set in the if
> statement rather than at the top of the function.
> 

+1 on "programmer to analyze their code fully in light of the warning"

I am not necessarily referring to the clang warning. I agree that it could
be misleading and suggest or send us towards a wrong fix. It is on us to
be careful - this would be when fixing the problem and reviewing the fix.

thanks,
-- Shuah


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2022-07-01 16:59 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-01  8:49 [char-misc:char-misc-linus 3/3] drivers/misc/cardreader/rtsx_usb.c:639:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is true kernel test robot
2022-07-01  8:59 ` Greg Kroah-Hartman
2022-07-01  8:59   ` Greg Kroah-Hartman
2022-07-01 14:48   ` Shuah Khan
2022-07-01 14:48     ` Shuah Khan
2022-07-01 15:39     ` Nathan Chancellor
2022-07-01 15:39       ` Nathan Chancellor
2022-07-01 15:52       ` Shuah Khan
2022-07-01 15:52         ` Shuah Khan
2022-07-01 16:09         ` Shuah Khan
2022-07-01 16:09           ` Shuah Khan
2022-07-01 16:27           ` Nathan Chancellor
2022-07-01 16:27             ` Nathan Chancellor
2022-07-01 16:59             ` Shuah Khan
2022-07-01 16:59               ` Shuah Khan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.