Re: [PATCH v4 1/2] media: rc: new driver for USB-UIRT device

On Fri, Jun 18, 2021 at 10:04:58AM -0700, Nathan Chancellor wrote:

On 6/18/2021 1:44 AM, Sean Young wrote:

quoted

On Fri, Jun 18, 2021 at 06:18:06AM +0800, kernel test robot wrote:

quoted

Hi Sean,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on usb-serial/usb-next usb/usb-testing peter.chen-usb/for-usb-next v5.13-rc6 next-20210617]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
base:   git://linuxtv.org/media_tree.git master
config: powerpc64-randconfig-r012-20210617 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 64720f57bea6a6bf033feef4a5751ab9c0c3b401)
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 powerpc64 cross compiling tool for clang build
         # apt-get install binutils-powerpc64-linux-gnu
         # https://github.com/0day-ci/linux/commit/17d3a0332baecb0359e05e8ae755478c7a1a4468
         git remote add linux-review https://github.com/0day-ci/linux
         git fetch --no-tags linux-review Sean-Young/IR-driver-for-USB-UIRT-device/20210616-182135
         git checkout 17d3a0332baecb0359e05e8ae755478c7a1a4468
         # save the attached .config to linux build tree
         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=powerpc64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <redacted>

All warnings (new ones prefixed by >>):

    In file included from drivers/media/rc/uirt.c:11:
    In file included from include/linux/completion.h:12:
    In file included from include/linux/swait.h:5:
    In file included from include/linux/list.h:9:
    In file included from include/linux/kernel.h:12:
    In file included from include/linux/bitops.h:32:
    In file included from arch/powerpc/include/asm/bitops.h:62:
    arch/powerpc/include/asm/barrier.h:49:9: warning: '__lwsync' macro redefined [-Wmacro-redefined]
    #define __lwsync()      __asm__ __volatile__ (stringify_in_c(LWSYNC) : : :"memory")
            ^
    <built-in>:310:9: note: previous definition is here
    #define __lwsync __builtin_ppc_lwsync
            ^

quoted

quoted

drivers/media/rc/uirt.c:639:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]

            if (!urb)
                ^~~~
    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
            return err;

This is interesting. clang is right here, there are error paths where err is
not initialized. gcc-11.1 does not pick this up for some reason. The error path
should be an immediate dominator so it shouldn't be complicated to detect.

The reason GCC does not warn about this is due to commit 78a5255ffb6a ("Stop
the ad-hoc games with -Wno-maybe-initialized"), which disables the GCC
version of this warning except with W=2, which very few people use. You
could use 'KCFLAGS=-Wmaybe-uninitialized' to try and see the same warning.

You're right, this is exactly the issue here.  When running with W=2 or
KCFLAGS=-Wmaybe-uninitialized you do get the warning.

There are paths where err are initialized, so gcc correctly says:

warning: ‘err’ may be used uninitialized in this function [-Wmaybe-uninitialized]


Thanks,

Sean

Cheers,
Nathan

quoted

I'll send out a v5 with this issue fixed.

Sean

quoted

                   ^~~
    drivers/media/rc/uirt.c:639:2: note: remove the 'if' if its condition is always false
            if (!urb)
            ^~~~~~~~~
    drivers/media/rc/uirt.c:630:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
            if (!urb)
                ^~~~
    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
            return err;
                   ^~~
    drivers/media/rc/uirt.c:630:2: note: remove the 'if' if its condition is always false
            if (!urb)
            ^~~~~~~~~
    drivers/media/rc/uirt.c:626:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
            if (!rc)
                ^~~
    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
            return err;
                   ^~~
    drivers/media/rc/uirt.c:626:2: note: remove the 'if' if its condition is always false
            if (!rc)
            ^~~~~~~~
    drivers/media/rc/uirt.c:622:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
            if (!uirt->out)
                ^~~~~~~~~~
    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
            return err;
                   ^~~
    drivers/media/rc/uirt.c:622:2: note: remove the 'if' if its condition is always false
            if (!uirt->out)
            ^~~~~~~~~~~~~~~
    drivers/media/rc/uirt.c:618:6: warning: variable 'err' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
            if (!uirt->in)
                ^~~~~~~~~
    drivers/media/rc/uirt.c:705:9: note: uninitialized use occurs here
            return err;
                   ^~~
    drivers/media/rc/uirt.c:618:2: note: remove the 'if' if its condition is always false
            if (!uirt->in)
            ^~~~~~~~~~~~~~
    drivers/media/rc/uirt.c:604:15: note: initialize the variable 'err' to silence this warning
            int pipe, err;
                         ^
                          = 0
    6 warnings generated.


vim +639 drivers/media/rc/uirt.c

    594	
    595	static int uirt_probe(struct usb_interface *intf,
    596			      const struct usb_device_id *id)
    597	{
    598		struct usb_device *usbdev = interface_to_usbdev(intf);
    599		struct usb_endpoint_descriptor *ep_in;
    600		struct usb_endpoint_descriptor *ep_out;
    601		struct uirt *uirt;
    602		struct rc_dev *rc;
    603		struct urb *urb;
    604		int pipe, err;
    605	
    606		if (usb_find_common_endpoints(intf->cur_altsetting, &ep_in, &ep_out, NULL, NULL) ||
    607		    usb_endpoint_maxp(ep_in) != MAX_PACKET ||
    608		    usb_endpoint_maxp(ep_out) != MAX_PACKET) {
    609			dev_err(&intf->dev, "required endpoints not found\n");
    610			return -ENODEV;
    611		}
    612	
    613		uirt = kzalloc(sizeof(*uirt), GFP_KERNEL);
    614		if (!uirt)
    615			return -ENOMEM;
    616	
    617		uirt->in = kmalloc(MAX_PACKET, GFP_KERNEL);
    618		if (!uirt->in)
    619			goto free_uirt;
    620	
    621		uirt->out = kmalloc(MAX_PACKET, GFP_KERNEL);
    622		if (!uirt->out)
    623			goto free_uirt;
    624	
    625		rc = rc_allocate_device(RC_DRIVER_IR_RAW);
    626		if (!rc)
    627			goto free_uirt;
    628	
    629		urb = usb_alloc_urb(0, GFP_KERNEL);
    630		if (!urb)
    631			goto free_rcdev;
    632	
    633		pipe = usb_rcvbulkpipe(usbdev, ep_in->bEndpointAddress);
    634		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->in, MAX_PACKET,
    635				  uirt_in_callback, uirt);
    636		uirt->urb_in = urb;
    637	
    638		urb = usb_alloc_urb(0, GFP_KERNEL);
  > 639		if (!urb)
    640			goto free_rcdev;
    641	
    642		pipe = usb_sndbulkpipe(usbdev, ep_out->bEndpointAddress);
    643		usb_fill_bulk_urb(urb, usbdev, pipe, uirt->out, MAX_PACKET,
    644				  uirt_out_callback, uirt);
    645	
    646		uirt->dev = &intf->dev;
    647		uirt->usbdev = usbdev;
    648		uirt->rc = rc;
    649		uirt->urb_out = urb;
    650		uirt->rx_state = RX_STATE_INTERSPACE_HIGH;
    651	
    652		err = usb_submit_urb(uirt->urb_in, GFP_KERNEL);
    653		if (err != 0) {
    654			dev_err(uirt->dev, "failed to submit read urb: %d\n", err);
    655			goto free_rcdev;
    656		}
    657	
    658		err = init_ftdi(usbdev);
    659		if (err) {
    660			dev_err(uirt->dev, "failed to setup ftdi: %d\n", err);
    661			goto kill_urbs;
    662		}
    663	
    664		err = uirt_setup(uirt);
    665		if (err)
    666			goto kill_urbs;
    667	
    668		usb_make_path(usbdev, uirt->phys, sizeof(uirt->phys));
    669	
    670		rc->device_name = "USB-UIRT";
    671		rc->driver_name = KBUILD_MODNAME;
    672		rc->input_phys = uirt->phys;
    673		usb_to_input_id(usbdev, &rc->input_id);
    674		rc->dev.parent = &intf->dev;
    675		rc->priv = uirt;
    676		rc->tx_ir = uirt_tx;
    677		rc->s_tx_carrier = uirt_set_tx_carrier;
    678		rc->s_learning_mode = uirt_set_rx_wideband;
    679		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
    680		rc->map_name = RC_MAP_RC6_MCE;
    681		rc->rx_resolution = UNIT_US;
    682		rc->timeout = IR_TIMEOUT;
    683	
    684		uirt_set_tx_carrier(rc, 38000);
    685	
    686		err = rc_register_device(rc);
    687		if (err)
    688			goto kill_urbs;
    689	
    690		usb_set_intfdata(intf, uirt);
    691	
    692		return 0;
    693	
    694	kill_urbs:
    695		usb_kill_urb(uirt->urb_in);
    696		usb_kill_urb(uirt->urb_out);
    697	free_rcdev:
    698		usb_free_urb(uirt->urb_in);
    699		usb_free_urb(uirt->urb_out);
    700		rc_free_device(rc);
    701	free_uirt:
    702		kfree(uirt->in);
    703		kfree(uirt->out);
    704		kfree(uirt);
    705		return err;
    706	}
    707	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org