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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1012388b-f316-47a3-a98f-0f40dc74948e@stanley.mountain>
Date: Wed, 4 Dec 2024 20:21:02 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Dave Penkler <dpenkler@...il.com>
Cc: gregkh@...uxfoundation.org, linux-staging@...ts.linux.dev,
	linux-kernel@...r.kernel.org,
	Marcello Carla' <marcello.carla@....com>
Subject: Re: [PATCH 3/4 v2] staging: gpib: Fix erroneous removal of blank
 before newline

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