linux-kernel - Re: [PATCH] staging: gpib: Make static and reduce forward declarations

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <6889992d-043d-4af8-869a-84eea9609148@stanley.mountain>
Date: Wed, 22 Jan 2025 10:37:33 +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
Subject: Re: [PATCH] staging: gpib: Make static and reduce forward
 declarations

On Tue, Jan 21, 2025 at 09:33:42PM +0100, Dave Penkler wrote:
> Declaring the entry points as static caused a warning that the
> serial_poll_status function of the agilent_82350b driver was
> unused.
> 
> Add the entry point to the corresponding interface structure
> initializations where it was missing.

...

> @@ -842,6 +824,7 @@ static gpib_interface_t agilent_82350b_unaccel_interface = {
>  	.primary_address = agilent_82350b_primary_address,
>  	.secondary_address = agilent_82350b_secondary_address,
>  	.serial_poll_response = agilent_82350b_serial_poll_response,
> +	.serial_poll_status = agilent_82350b_serial_poll_status,
>  	.t1_delay = agilent_82350b_t1_delay,
>  	.return_to_local = agilent_82350b_return_to_local,
>  };
> @@ -869,12 +852,12 @@ static gpib_interface_t agilent_82350b_interface = {
>  	.primary_address = agilent_82350b_primary_address,
>  	.secondary_address = agilent_82350b_secondary_address,
>  	.serial_poll_response = agilent_82350b_serial_poll_response,
> +	.serial_poll_status = agilent_82350b_serial_poll_status,
>  	.t1_delay = agilent_82350b_t1_delay,
>  	.return_to_local = agilent_82350b_return_to_local,
>  };

So what happened is that Sparse was complaining and you were cleaning
up the code and you discovered this bug.  Fine.  But bug fixes need to
be in their own commit, not hidden inside a giant cleanup patch.  They
need to have a commit message.

Quite often it sucks to discover a bug like this because the bugs have
to be fixed first before the cleanup.  We're a bit less strict on this
in staging because realistically there are lots of bugs and lots of
cleanups and they're going to get mixed together.  But other subsystems
maintain a fixes branch and a new development branch and so bug fixes
have to apply cleanly to the fixes branch.

So in that case you would need to save the diff of the cleanup.  Go back
to the start.  Write the fix.  Apply the cleanup diff on the top.  Deal
with any failed chunks.  What a headache!  I get that.  And you're from
a gmail.com address so I don't even know if you're getting paid to deal
with this crap...

Quite often it sucks but in this case, it's really easy.  Use
`git citool`, highlight the bugfix lines, right click, add to commit
write a commit message and done.  Make sure it still builds as a stand
alone patch, though.  Quite often when I try to highlight and click on
all the lines in a bugfix, I accidentally leave out essential pieces.

regards,
dan carpenter