RE: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-17 Thread Maciej W. Rozycki
On Sat, 17 Apr 2021, David Laight wrote:

> > > In patch 2, vscnprintf should probably be used to make sure it's
> > > 0 terminated.
> > 
> >  Why?  C99 has this[1]:
> > 
> > "The vsnprintf function is equivalent to snprintf, with the variable
> > argument list replaced by arg, which shall have been initialized by the
> > va_start macro (and possibly subsequent va_arg calls)."
> 
> vscnprintf() is normally the function you want (not vsnprintf())
> because the return value is the number of characters actually
> put into the buffer, not the number that would have been written
> had the buffer been long enough.

 Good catch, thank you!  I'll respin the series then.  Thank you for the 
background story too!

  Maciej


RE: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-17 Thread David Laight
From: Maciej W. Rozycki
> Sent: 16 April 2021 11:49
> 
> On Thu, 15 Apr 2021, Joe Perches wrote:
> 
> > In patch 2, vscnprintf should probably be used to make sure it's
> > 0 terminated.
> 
>  Why?  C99 has this[1]:
> 
> "The vsnprintf function is equivalent to snprintf, with the variable
> argument list replaced by arg, which shall have been initialized by the
> va_start macro (and possibly subsequent va_arg calls)."

vscnprintf() is normally the function you want (not vsnprintf())
because the return value is the number of characters actually
put into the buffer, not the number that would have been written
had the buffer been long enough.
Return values larger than the buffer size are almost never
allowed for - and are probably a set of 'buffer overflow' bugs.

While probably justified by saying that it lets you malloc()
a big enough buffer and try again, the return value is almost
certainly just historic.

The original sprintf() libc code allocated a FILE structure on
stack set to fully-buffered with the current buffer pointer set
to the caller's buffer and a buffer length of MAXINT.
It then just called vprintf() to do the work.

snprintf() was done the same way, except the buffer length was
set and the 'write character' (or 'flush buffer') function
intercepted to avoid writes beyond the buffer end.
(Possibly by re-routing the writes to a global buffer.)
The return value from vprintf() gets returned to the user.

The Unix versions have always '\0' terminated the buffer.
Only Microsoft has ever released an snprintf() that doesn't
'\0' terminate the output - another source of bugs.

Personally I think bounded string functions should
return the buffer size on overflow.
This means sequences of:
offset += xxx(buf + offset, sizeof buf - offset, ...);
are safe and the overflow can be detected right at the end.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, 
UK
Registration No: 1397386 (Wales)



Re: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-16 Thread Joe Perches
On Fri, 2021-04-16 at 14:41 -0600, Khalid Aziz wrote:
> On 4/15/21 8:08 PM, Joe Perches wrote:
> > And while it's a lot more code, I'd prefer a solution that looks more
> > like the other commonly used kernel logging extension mechanisms
> > where adapter is placed before the format, ... in the argument list.
> 
> Hi Joe,
> 
> I don't mind making these changes. It is quite a bit of code but
> consistency with other kernel code is useful. Would you like to finalize
> this patch, or would you prefer that I take this patch as starting point
> and finalize it?

Probably better to apply Maciej's patches first and then something
like this.

btw: the coccinelle script was

@@
expression a, b;
@@


\(blogic_announce\|blogic_info\|blogic_notice\|blogic_warn\|blogic_err\)(
-   a, b
+   b, a
, ...)




Re: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-16 Thread Khalid Aziz
On 4/15/21 8:08 PM, Joe Perches wrote:
> And while it's a lot more code, I'd prefer a solution that looks more
> like the other commonly used kernel logging extension mechanisms
> where adapter is placed before the format, ... in the argument list.

Hi Joe,

I don't mind making these changes. It is quite a bit of code but
consistency with other kernel code is useful. Would you like to finalize
this patch, or would you prefer that I take this patch as starting point
and finalize it?

Thanks,
Khalid

> 
> Today it's:
> 
> void blogic_msg(enum, fmt, adapter, ...);
> 
> without the __printf marking so there is one format/arg mismatch.
> 
> fyi: in the suggested patch below it's
> - blogic_info("BIOS Address: 0x%lX, ", adapter,
> - adapter->bios_addr);
> + blogic_cont(adapter, "BIOS Address: 0x%X, ",
> + adapter->bios_addr);
> 
> I'd prefer
> __printf(3, 4)
> void blogic_msg(enum, adapter, fmt, ...)
> 
> (or maybe void blogic_msg(adapter, enum, fmt, ...))
> 
> And there's a simple addition of a blogic_cont macro and extension
> to blogic_msg to simplify the logic and obviousness of the logging
> extension lines too.
> 
> I suggest this done with coccinelle and a little typing:
> ---
>  drivers/scsi/BusLogic.c | 496 
> +++-
>  drivers/scsi/BusLogic.h |  32 ++--
>  2 files changed, 341 insertions(+), 187 deletions(-)
> 
> diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
> index ccb061ab0a0a..7a52371b5ab6 100644
> --- a/drivers/scsi/BusLogic.c
> +++ b/drivers/scsi/BusLogic.c
> @@ -134,8 +134,10 @@ static char *blogic_cmd_failure_reason;
>  
>  static void blogic_announce_drvr(struct blogic_adapter *adapter)
>  {
> - blogic_announce("* BusLogic SCSI Driver Version " 
> blogic_drvr_version " of " blogic_drvr_date " *\n", adapter);
> - blogic_announce("Copyright 1995-1998 by Leonard N. Zubkoff 
> \n", adapter);
> + blogic_announce(adapter,
> + "* BusLogic SCSI Driver Version " 
> blogic_drvr_version " of " blogic_drvr_date " *\n");
> + blogic_announce(adapter,
> + "Copyright 1995-1998 by Leonard N. Zubkoff 
> \n");
>  }
>  
>  
> @@ -198,8 +200,7 @@ static bool __init blogic_create_initccbs(struct 
> blogic_adapter *adapter)
>   blk_pointer = dma_alloc_coherent(&adapter->pci_device->dev,
>   blk_size, &blkp, GFP_KERNEL);
>   if (blk_pointer == NULL) {
> - blogic_err("UNABLE TO ALLOCATE CCB GROUP - DETACHING\n",
> - adapter);
> + blogic_err(adapter, "UNABLE TO ALLOCATE CCB GROUP - 
> DETACHING\n");
>   return false;
>   }
>   blogic_init_ccbs(adapter, blk_pointer, blk_size, blkp);
> @@ -259,10 +260,13 @@ static void blogic_create_addlccbs(struct 
> blogic_adapter *adapter,
>   }
>   if (adapter->alloc_ccbs > prev_alloc) {
>   if (print_success)
> - blogic_notice("Allocated %d additional CCBs (total now 
> %d)\n", adapter, adapter->alloc_ccbs - prev_alloc, adapter->alloc_ccbs);
> + blogic_notice(adapter,
> +   "Allocated %d additional CCBs (total now 
> %d)\n",
> +   adapter->alloc_ccbs - prev_alloc,
> +   adapter->alloc_ccbs);
>   return;
>   }
> - blogic_notice("Failed to allocate additional CCBs\n", adapter);
> + blogic_notice(adapter, "Failed to allocate additional CCBs\n");
>   if (adapter->drvr_qdepth > adapter->alloc_ccbs - adapter->tgt_count) {
>   adapter->drvr_qdepth = adapter->alloc_ccbs - adapter->tgt_count;
>   adapter->scsi_host->can_queue = adapter->drvr_qdepth;
> @@ -441,7 +445,9 @@ static int blogic_cmd(struct blogic_adapter *adapter, 
> enum blogic_opcode opcode,
>   goto done;
>   }
>   if (blogic_global_options.trace_config)
> - blogic_notice("blogic_cmd(%02X) Status = %02X: (Modify 
> I/O Address)\n", adapter, opcode, statusreg.all);
> + blogic_notice(adapter,
> +   "blogic_cmd(%02X) Status = %02X: (Modify 
> I/O Address)\n",
> +   opcode, statusreg.all);
>   result = 0;
>   goto done;
>   }
> @@ -499,15 +505,16 @@ static int blogic_cmd(struct blogic_adapter *adapter, 
> enum blogic_opcode opcode,
>*/
>   if (blogic_global_options.trace_config) {
>   int i;
> - blogic_notice("blogic_cmd(%02X) Status = %02X: %2d ==> %2d:",
> - adapter, opcode, statusreg.all, replylen,
> + blogic_notice(adapter,
> +   "blogic_cmd(%02X) Stat

Re: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-16 Thread Khalid Aziz
On 4/14/21 4:39 PM, Maciej W. Rozycki wrote:
> Update BusLogic driver's messaging system to use `pr_cont' for 
> continuation lines, bringing messy output:
> 
> pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0:   PCI Bus: 0, Device: 19, Address:
> 0xE0012000,
> Host Adapter SCSI ID: 7
> scsi0:   Parity Checking: Enabled, Extended Translation: Enabled
> scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0:   Tagged Queue Depth:
> Automatic
> , Untagged Queue Depth: 3
> scsi0:   SCSI Bus Termination: Both Enabled
> , SCAM: Disabled
> 
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
> 
> back to order:
> 
> pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0:   PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7
> scsi0:   Parity Checking: Enabled, Extended Translation: Enabled
> scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0:   Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
> scsi0:   SCSI Bus Termination: Both Enabled, SCAM: Disabled
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
> 
> Also diagnostic output such as with the `BusLogic=TraceConfiguration' 
> parameter is affected and becomes vertical and therefore hard to read.  
> This has now been corrected, e.g.:
> 
> pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00
> blogic_cmd(95) Status = 28: (Modify I/O Address)
> blogic_cmd(91) Status = 30:  1 ==>  1: 01
> blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 
> 1D
> scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
> blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30
> blogic_cmd(0B) Status = 30:  3 ==>  3: 00 08 07
> blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 
> 00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 
> 1D
> blogic_cmd(84) Status = 30:  1 ==>  1: 37
> blogic_cmd(8B) Status = 30:  5 ==>  5: 39 35 38 20 20
> blogic_cmd(85) Status = 30:  1 ==>  1: 42
> blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00
> blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 
> 01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 
> 00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> 
> etc.
> 
> Signed-off-by: Maciej W. Rozycki 
> Fixes: 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation 
> lines")
> Cc: sta...@vger.kernel.org # v4.9+
> ---
>  drivers/scsi/BusLogic.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> linux-buslogic-pr-cont.diff
> Index: linux-macro-ide/drivers/scsi/BusLogic.c
> ===
> --- linux-macro-ide.orig/drivers/scsi/BusLogic.c
> +++ linux-macro-ide/drivers/scsi/BusLogic.c
> @@ -3603,7 +3603,7 @@ static void blogic_msg(enum blogic_msgle
>   if (buf[0] != '\n' || len > 1)
>   printk("%sscsi%d: %s", 
> blogic_msglevelmap[msglevel], adapter->host_no, buf);
>   } else
> - printk("%s", buf);
> + pr_cont("%s", buf);
>   } else {
>   if (begin) {
>   if (adapter != NULL && adapter->adapter_initd)
> @@ -3611,7 +3611,7 @@ static void blogic_msg(enum blogic_msgle
>   else
>   printk("%s%s", blogic_msglevelmap[msglevel], 
> buf);
>   } else
> - printk("%s", buf);
> + pr_cont("%s", buf);
>   }
>   begin = (buf[len - 1] == '\n');
>  }
> 

Look

Re: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-16 Thread Joe Perches
On Fri, 2021-04-16 at 16:28 +0200, Maciej W. Rozycki wrote:
> On Fri, 16 Apr 2021, Joe Perches wrote:
> 
> > > I'm not sure if that complex message 
> > > routing via `blogic_msg' is worth having even, rather than calling 
> > > `printk' or suitable variants directly.
> > 
> > It's to allow the message content to be added to the internal
> > &adapter->msgbuf[adapter->msgbuflen]
> > with strcpy for later use with blogic_show_info()/seq_write.
> 
>  I know, but it's not clear to me if it's worth it (a potential buffer 
> overrun there too, BTW).

It's seq_ output so it's nominally an ABI.
But then again, I don't use this at all so I don't care much either.

It's also odd/bad form that one output KERN_ does not match
its blogic_ (blogic_info is emitted at KERN_NOTICE)





Re: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-16 Thread Maciej W. Rozycki
On Fri, 16 Apr 2021, Joe Perches wrote:

> > I'm not sure if that complex message 
> > routing via `blogic_msg' is worth having even, rather than calling 
> > `printk' or suitable variants directly.
> 
> It's to allow the message content to be added to the internal
>   &adapter->msgbuf[adapter->msgbuflen]
> with strcpy for later use with blogic_show_info()/seq_write.

 I know, but it's not clear to me if it's worth it (a potential buffer 
overrun there too, BTW).

  Maciej


Re: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-16 Thread Joe Perches
On Fri, 2021-04-16 at 12:48 +0200, Maciej W. Rozycki wrote:
> I'm not sure if that complex message 
> routing via `blogic_msg' is worth having even, rather than calling 
> `printk' or suitable variants directly.

It's to allow the message content to be added to the internal
&adapter->msgbuf[adapter->msgbuflen]
with strcpy for later use with blogic_show_info()/seq_write.





Re: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-16 Thread Maciej W. Rozycki
On Thu, 15 Apr 2021, Joe Perches wrote:

> In patch 2, vscnprintf should probably be used to make sure it's
> 0 terminated.

 Why?  C99 has this[1]:

"The vsnprintf function is equivalent to snprintf, with the variable 
argument list replaced by arg, which shall have been initialized by the 
va_start macro (and possibly subsequent va_arg calls)."

and then[2]:

"The snprintf function is equivalent to fprintf, except that the output 
is written into an array (specified by argument s) rather than to a 
stream.  If n is zero, nothing is written, and s may be a null pointer. 
Otherwise, output characters beyond the n-1st are discarded rather than 
being written to the array, and a null character is written at the end 
of the characters actually written into the array."

therefore output from `vsnprintf' is always null-terminated.

> And while it's a lot more code, I'd prefer a solution that looks more
> like the other commonly used kernel logging extension mechanisms
> where adapter is placed before the format, ... in the argument list.

 I agree having `adapter' as the second argument seems weird, so that is 
fine with me as a follow-up cleanup.  However as a user-visible change I 
think the fix I propose here ought to be applied first (and backported 
as suitable).  Then any internal clean-ups can follow, applied to trunk 
only.

> And there's a simple addition of a blogic_cont macro and extension
> to blogic_msg to simplify the logic and obviousness of the logging
> extension lines too.

 I did this first actually, before I realised a simpler change suitable 
for backporting could be done.  I'm not sure if that complex message 
routing via `blogic_msg' is worth having even, rather than calling 
`printk' or suitable variants directly.

References:

[1] "Programming languages -- C", INTERNATIONAL STANDARD, ISO/IEC 9899, 
Second edition, 1999-12-01, Section 7.19.6.12 "The vsnprintf 
function", p.293

[2] same, 7.19.6.5 "The snprintf function", p.289

  Maciej


Re: [PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-15 Thread Joe Perches
On Thu, 2021-04-15 at 00:39 +0200, Maciej W. Rozycki wrote:
> Update BusLogic driver's messaging system to use `pr_cont' for 
> continuation lines, bringing messy output:
> 
> pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0:   PCI Bus: 0, Device: 19, Address:
> 0xE0012000,
> Host Adapter SCSI ID: 7
> scsi0:   Parity Checking: Enabled, Extended Translation: Enabled
> scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0:   Tagged Queue Depth:
> Automatic
> , Untagged Queue Depth: 3
> scsi0:   SCSI Bus Termination: Both Enabled
> , SCAM: Disabled
> 
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
> 
> back to order:
> 
> pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
> scsi0:   PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7
> scsi0:   Parity Checking: Enabled, Extended Translation: Enabled
> scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
> scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
> scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
> scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192
> scsi0:   Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
> scsi0:   SCSI Bus Termination: Both Enabled, SCAM: Disabled
> scsi0: *** BusLogic BT-958 Initialized Successfully ***
> scsi host0: BusLogic BT-958
> 
> Also diagnostic output such as with the `BusLogic=TraceConfiguration' 
> parameter is affected and becomes vertical and therefore hard to read.  
> This has now been corrected, e.g.:
> 
> pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
> blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00
> blogic_cmd(95) Status = 28: (Modify I/O Address)
> blogic_cmd(91) Status = 30:  1 ==>  1: 01
> blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 
> 1D
> scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
> scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
> blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30
> blogic_cmd(0B) Status = 30:  3 ==>  3: 00 08 07
> blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 
> 00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00
> blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 
> 1D
> blogic_cmd(84) Status = 30:  1 ==>  1: 37
> blogic_cmd(8B) Status = 30:  5 ==>  5: 39 35 38 20 20
> blogic_cmd(85) Status = 30:  1 ==>  1: 42
> blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00
> blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 
> 01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 
> 00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC
> scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
> 
> etc.

In patch 2, vscnprintf should probably be used to make sure it's
0 terminated.

And while it's a lot more code, I'd prefer a solution that looks more
like the other commonly used kernel logging extension mechanisms
where adapter is placed before the format, ... in the argument list.

Today it's:

void blogic_msg(enum, fmt, adapter, ...);

without the __printf marking so there is one format/arg mismatch.

fyi: in the suggested patch below it's
-   blogic_info("BIOS Address: 0x%lX, ", adapter,
-   adapter->bios_addr);
+   blogic_cont(adapter, "BIOS Address: 0x%X, ",
+   adapter->bios_addr);

I'd prefer
__printf(3, 4)
void blogic_msg(enum, adapter, fmt, ...)

(or maybe void blogic_msg(adapter, enum, fmt, ...))

And there's a simple addition of a blogic_cont macro and extension
to blogic_msg to simplify the logic and obviousness of the logging
extension lines too.

I suggest this done with coccinelle and a little typing:
---
 drivers/scsi/BusLogic.c | 496 +++-
 drivers/scsi/BusLogic.h |  32 ++--
 2 files changed, 341 insertions(+), 187 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index ccb061ab0a0a..7a52371b5

[PATCH 1/5] scsi: BusLogic: Fix missing `pr_cont' use

2021-04-14 Thread Maciej W. Rozycki
Update BusLogic driver's messaging system to use `pr_cont' for 
continuation lines, bringing messy output:

pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
scsi0:   PCI Bus: 0, Device: 19, Address:
0xE0012000,
Host Adapter SCSI ID: 7
scsi0:   Parity Checking: Enabled, Extended Translation: Enabled
scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192
scsi0:   Tagged Queue Depth:
Automatic
, Untagged Queue Depth: 3
scsi0:   SCSI Bus Termination: Both Enabled
, SCAM: Disabled

scsi0: *** BusLogic BT-958 Initialized Successfully ***
scsi host0: BusLogic BT-958

back to order:

pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter
scsi0:   Firmware Version: 5.07B, I/O Address: 0x7000, IRQ Channel: 17/Level
scsi0:   PCI Bus: 0, Device: 19, Address: 0xE0012000, Host Adapter SCSI ID: 7
scsi0:   Parity Checking: Enabled, Extended Translation: Enabled
scsi0:   Synchronous Negotiation: Ultra, Wide Negotiation: Enabled
scsi0:   Disconnect/Reconnect: Enabled, Tagged Queuing: Enabled
scsi0:   Scatter/Gather Limit: 128 of 8192 segments, Mailboxes: 211
scsi0:   Driver Queue Depth: 211, Host Adapter Queue Depth: 192
scsi0:   Tagged Queue Depth: Automatic, Untagged Queue Depth: 3
scsi0:   SCSI Bus Termination: Both Enabled, SCAM: Disabled
scsi0: *** BusLogic BT-958 Initialized Successfully ***
scsi host0: BusLogic BT-958

Also diagnostic output such as with the `BusLogic=TraceConfiguration' 
parameter is affected and becomes vertical and therefore hard to read.  
This has now been corrected, e.g.:

pci :00:13.0: PCI->APIC IRQ transform: INT A -> IRQ 17
blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00
blogic_cmd(95) Status = 28: (Modify I/O Address)
blogic_cmd(91) Status = 30:  1 ==>  1: 01
blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30
blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
scsi: * BusLogic SCSI Driver Version 2.1.17 of 12 September 2013 *
scsi: Copyright 1995-1998 by Leonard N. Zubkoff 
blogic_cmd(04) Status = 30:  4 ==>  4: 41 41 35 30
blogic_cmd(0B) Status = 30:  3 ==>  3: 00 08 07
blogic_cmd(0D) Status = 30: 34 ==> 34: 03 01 07 04 00 00 00 00 00 00 00 00 00 
00 00 00 FF 42 44 46 FF 00 00 00 00 00 00 00 00 00 FF 00 FF 00
blogic_cmd(8D) Status = 30: 14 ==> 14: 45 DC 00 20 00 00 00 00 00 40 30 37 42 1D
blogic_cmd(84) Status = 30:  1 ==>  1: 37
blogic_cmd(8B) Status = 30:  5 ==>  5: 39 35 38 20 20
blogic_cmd(85) Status = 30:  1 ==>  1: 42
blogic_cmd(86) Status = 30:  4 ==>  4: FF 05 93 00
blogic_cmd(91) Status = 30: 64 ==> 64: 41 46 3E 20 39 35 38 20 20 00 C4 00 04 
01 07 2F 07 04 35 FF FF FF FF FF FF FF FF FF FF 01 00 FE FF 08 FF FF 00 00 00 
00 00 00 00 01 00 01 00 00 FF FF 00 00 00 00 00 00 00 00 00 00 00 00 00 FC
scsi0: Configuring BusLogic Model BT-958 PCI Wide Ultra SCSI Host Adapter

etc.

Signed-off-by: Maciej W. Rozycki 
Fixes: 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation 
lines")
Cc: sta...@vger.kernel.org # v4.9+
---
 drivers/scsi/BusLogic.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

linux-buslogic-pr-cont.diff
Index: linux-macro-ide/drivers/scsi/BusLogic.c
===
--- linux-macro-ide.orig/drivers/scsi/BusLogic.c
+++ linux-macro-ide/drivers/scsi/BusLogic.c
@@ -3603,7 +3603,7 @@ static void blogic_msg(enum blogic_msgle
if (buf[0] != '\n' || len > 1)
printk("%sscsi%d: %s", 
blogic_msglevelmap[msglevel], adapter->host_no, buf);
} else
-   printk("%s", buf);
+   pr_cont("%s", buf);
} else {
if (begin) {
if (adapter != NULL && adapter->adapter_initd)
@@ -3611,7 +3611,7 @@ static void blogic_msg(enum blogic_msgle
else
printk("%s%s", blogic_msglevelmap[msglevel], 
buf);
} else
-   printk("%s", buf);
+   pr_cont("%s", buf);
}
begin = (buf[len - 1] == '\n');
 }