[PATCH 3/4 v2] staging: gpib: Fix erroneous removal of blank before newline

[PATCH 3/4 v2] staging: gpib: Fix erroneous removal of blank before newline

[Dave Penkler]

The original commit removed the blanks before the newline
in the protocol string constants to satisfy checkpatch.pl
This broke the driver since it relies on the correct length
of the string constants including the blank.
For example the original
  #define USB_GPIB_SET_LINES   "\nIBDC \n"
became
  #define USB_GPIB_SET_LINES   "\nIBDC\n"
which broke the driver.

The solution is to replace original blanks in protocol constants
with "."
e.g.:
  #define USB_GPIB_SET_LINES   "\nIBDC.\n"

Reported-by: Marcello Carla' <marcello.carla@gmx.com>
Fixes: fce79512a96a ("staging: gpib: Add LPVO DIY USB GPIB driver")
Co-developed-by: Marcello Carla' <marcello.carla@gmx.com>
Signed-off-by: Marcello Carla' <marcello.carla@gmx.com>
Signed-off-by: Dave Penkler <dpenkler@gmail.com>
---
v1 -> v2
  Add more details to the commit message
  Add Signed-off by co developer
  Add spaces around assignment
  Fix tabs
  
 drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c b/drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c
index 1a8eb3bfb61c..81c110f29e76 100644
--- a/drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c
+++ b/drivers/staging/gpib/lpvo_usb_gpib/lpvo_usb_gpib.c
@@ -97,8 +97,8 @@ module_param(debug, int, 0644);
 #define USB_GPIB_DEBUG_ON    "\nIBDE\xAA\n"
 #define USB_GPIB_SET_LISTEN  "\nIBDT0\n"
 #define USB_GPIB_SET_TALK    "\nIBDT1\n"
-#define USB_GPIB_SET_LINES   "\nIBDC\n"
-#define USB_GPIB_SET_DATA    "\nIBDM\n"
+#define USB_GPIB_SET_LINES   "\nIBDC.\n"
+#define USB_GPIB_SET_DATA    "\nIBDM.\n"
 #define USB_GPIB_READ_LINES  "\nIBD?C\n"
 #define USB_GPIB_READ_DATA   "\nIBD?M\n"
 #define USB_GPIB_READ_BUS    "\nIBD??\n"
@@ -587,7 +587,7 @@ static int usb_gpib_command(gpib_board_t *board,
 			    size_t *bytes_written)
 {
 	int i, retval;
-	char command[6] = "IBc\n";
+	char command[6] = "IBc.\n";
 
 	DIA_LOG(1, "enter %p\n", board);
 
-- 
2.47.1

Re: [PATCH 3/4 v2] staging: gpib: Fix erroneous removal of blank before newline

[Dan Carpenter]

On Wed, Dec 04, 2024 at 06:04:43PM +0100, Dave Penkler wrote:
> The original commit removed the blanks before the newline
> in the protocol string constants to satisfy checkpatch.pl
> This broke the driver since it relies on the correct length
> of the string constants including the blank.
> For example the original
>   #define USB_GPIB_SET_LINES   "\nIBDC \n"
> became
>   #define USB_GPIB_SET_LINES   "\nIBDC\n"
> which broke the driver.
> 
> The solution is to replace original blanks in protocol constants
> with "."
> e.g.:
>   #define USB_GPIB_SET_LINES   "\nIBDC.\n"
> 

Let me help you write the commit message.

    The USB_GPIB_SET_LINES string used to be: "\nIBDC \n" but when we
    were merging this code into the upstream kernel we deleted the space
    character before the newline to make checkpatch happy.  That turned
    out to be a mistake.

    The "\nIBDC" part of the string is a command that we pass to the
    firmware and the next character is u8 value.  It gets set in
    set_control_line().

	msg[leng - 2] = value ? (retval & ~line) : retval | line;

    Imagine the parameter was supposed to be 8.
      Old: "\nIBDC8\n"
      New: "\nIBD8\n"

    The firmware doesn't recognize IBD as a valid command and [whatever
    starts beeping and sets the computer on fire].

    Put a . where the parameter is supposed to go which fixes the driver
    and makes checkpatch happy.  Same thing with the other defines.

regards,
dan carpenter