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