[bug report] staging: gpib: Add National Instruments USB GPIB driver

[bug report] staging: gpib: Add National Instruments USB GPIB driver

[Dan Carpenter]

Hello Dave Penkler,

Commit 4e127de14fa7 ("staging: gpib: Add National Instruments USB
GPIB driver") from Sep 18, 2024 (linux-next), leads to the following
Smatch static checker warning:

drivers/gpib/ni_usb/ni_usb_gpib.c:783 ni_usb_write() warn: set error code if 'usb_bytes_written != i'
drivers/gpib/ni_usb/ni_usb_gpib.c:882 ni_usb_command_chunk() warn: set error code if 'bytes_written != i'
drivers/gpib/ni_usb/ni_usb_gpib.c:981 ni_usb_take_control() warn: set error code if 'bytes_written != i'
drivers/gpib/ni_usb/ni_usb_gpib.c:1040 ni_usb_go_to_standby() warn: set error code if 'bytes_written != i'
drivers/gpib/ni_usb/ni_usb_gpib.c:1056 ni_usb_go_to_standby() warn: set error code if 'bytes_read != 12'
drivers/gpib/ni_usb/ni_usb_gpib.c:1384 ni_usb_parallel_poll() warn: set error code if 'bytes_written != i'
drivers/gpib/ni_usb/ni_usb_gpib.c:1555 ni_usb_line_status() warn: set error code if 'bytes_written != i'

drivers/gpib/ni_usb/ni_usb_gpib.c
    772         i += ni_usb_bulk_termination(&out_data[i]);
    773 
    774         mutex_lock(&ni_priv->addressed_transfer_lock);
    775 
    776         retval = ni_usb_send_bulk_msg(ni_priv, out_data, i, &usb_bytes_written,
    777                                       ni_usb_timeout_msecs(board->usec_timeout));
    778         kfree(out_data);
    779         if (retval || usb_bytes_written != i)        {
                              ^^^^^^^^^^^^^^^^^^^^^^

We should set an error code if the usb_bytes_written are not what we
expect.  But actually since most callers get this wrong it would be
better not return usb_bytes_written but instead just check that we
get what we expect in ni_usb_send_bulk_msg() and return a negative error
cdoe when it fails.

    780                 mutex_unlock(&ni_priv->addressed_transfer_lock);
    781                 dev_err(&usb_dev->dev, "send_bulk_msg returned %i, usb_bytes_written=%i, i=%i\n",
    782                         retval, usb_bytes_written, i);
--> 783                 return retval;
    784         }
    785 
    786         in_data = kmalloc(in_data_length, GFP_KERNEL);
    787         if (!in_data) {
    788                 mutex_unlock(&ni_priv->addressed_transfer_lock);
    789                 return -ENOMEM;
    790         }
    791         retval = ni_usb_receive_bulk_msg(ni_priv, in_data, in_data_length, &usb_bytes_read,
    792                                          ni_usb_timeout_msecs(board->usec_timeout), 1);
    793 
    794         mutex_unlock(&ni_priv->addressed_transfer_lock);
    795 
    796         if ((retval && retval != -ERESTARTSYS) || usb_bytes_read != 12) {
                                                          ^^^^^^^^^^^^^^^^^^^^
Same.  Except if retval is -ERESTARTSYS then probably usb_bytes_read is
not going to be 12 so that might be another issue as well.

    797                 dev_err(&usb_dev->dev, "receive_bulk_msg returned %i, usb_bytes_read=%i\n",
    798                         retval, usb_bytes_read);
    799                 kfree(in_data);
    800                 return retval;
    801         }
    802         ni_usb_parse_status_block(in_data, &status);
    803         kfree(in_data);
    804         switch        (status.error_code) {
    805         case NIUSB_NO_ERROR:
    806                 retval = 0;
    807                 break;
    808         case NIUSB_ABORTED_ERROR:
    809                 /*
    810                  * this is expected if ni_usb_receive_bulk_msg got

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter