diff mbox

[1/2] scsi: nsp32: add __printf attribute to logging functions

Message ID 20170520111628.24787-1-nicolas.iooss_linux@m4x.org (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

Nicolas Iooss May 20, 2017, 11:16 a.m. UTC 
nsp32_message() and nsp32_dmessage() use printf format strings in order
to format a message. Adding __printf attributes helps to detect errors
in such format strings at build time, like:

    drivers/scsi/nsp32.c:3314:23: error: format '%ld' expects argument
    of type 'long int', but argument 6 has type 'pm_message_t {aka
    struct pm_message}' [-Werror=format=]
      nsp32_msg(KERN_INFO,
      "pci-suspend: pdev=0x%p, state=%ld, slot=%s, host=0x%p",
      pdev, state, pci_name(pdev), host);

Fix all format string errors which were reported by gcc.

Signed-off-by: Nicolas Iooss <nicolas.iooss_linux@m4x.org>
---
 drivers/scsi/nsp32.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

Comments

Joe Perches May 20, 2017, 11:35 a.m. UTC | #1 
On Sat, 2017-05-20 at 13:16 +0200, Nicolas Iooss wrote:
> nsp32_message() and nsp32_dmessage() use printf format strings in order
> to format a message. Adding __printf attributes helps to detect errors
> in such format strings at build time, like:
> 
>     drivers/scsi/nsp32.c:3314:23: error: format '%ld' expects argument
>     of type 'long int', but argument 6 has type 'pm_message_t {aka
>     struct pm_message}' [-Werror=format=]
>       nsp32_msg(KERN_INFO,
>       "pci-suspend: pdev=0x%p, state=%ld, slot=%s, host=0x%p",
>       pdev, state, pci_name(pdev), host);
> 
> Fix all format string errors which were reported by gcc.
[]
> diff --git a/drivers/scsi/nsp32.c b/drivers/scsi/nsp32.c
[]
> @@ -321,7 +323,8 @@ static struct scsi_host_template nsp32_template = {
>  
>  #define NSP32_DEBUG_BUF_LEN		100
>  
> -static void nsp32_message(const char *func, int line, char *type, char *fmt, ...)
> +static __printf(4, 5)
> +void nsp32_message(const char *func, int line, char *type, const char *fmt, ...)
>  {
>  	va_list args;
>  	char buf[NSP32_DEBUG_BUF_LEN];

These could also use vsprintf extension %pV instead of
vsnprintf to a
temporary buffer and then using "%s, <tmp>"

etc...

Does anyone actually have or use these cards any longer?
Martin K. Petersen May 24, 2017, 2:02 a.m. UTC | #2 
Joe,

> On Sat, 2017-05-20 at 13:16 +0200, Nicolas Iooss wrote:
>> nsp32_message() and nsp32_dmessage() use printf format strings in order
>> to format a message. Adding __printf attributes helps to detect errors
>> in such format strings at build time, like:

> Does anyone actually have or use these cards any longer?

I doubt it.
diff mbox

Patch

diff --git a/drivers/scsi/nsp32.c b/drivers/scsi/nsp32.c
index 53c84771f0e8..0689b0564fbf 100644
--- a/drivers/scsi/nsp32.c
+++ b/drivers/scsi/nsp32.c
@@ -257,9 +257,11 @@  static        void nsp32_prom_set      (nsp32_hw_data *, int, int);
 static        int  nsp32_prom_get      (nsp32_hw_data *, int);
 
 /* debug/warning/info message */
-static void nsp32_message (const char *, int, char *, char *, ...);
+static __printf(4, 5)
+void nsp32_message (const char *, int, char *, const char *, ...);
 #ifdef NSP32_DEBUG
-static void nsp32_dmessage(const char *, int, int,    char *, ...);
+static __printf(4, 5)
+void nsp32_dmessage(const char *, int, int,    const char *, ...);
 #endif
 
 /*
@@ -321,7 +323,8 @@  static struct scsi_host_template nsp32_template = {
 
 #define NSP32_DEBUG_BUF_LEN		100
 
-static void nsp32_message(const char *func, int line, char *type, char *fmt, ...)
+static __printf(4, 5)
+void nsp32_message(const char *func, int line, char *type, const char *fmt, ...)
 {
 	va_list args;
 	char buf[NSP32_DEBUG_BUF_LEN];
@@ -338,7 +341,8 @@  static void nsp32_message(const char *func, int line, char *type, char *fmt, ...
 }
 
 #ifdef NSP32_DEBUG
-static void nsp32_dmessage(const char *func, int line, int mask, char *fmt, ...)
+static __printf(4, 5)
+void nsp32_dmessage(const char *func, int line, int mask, const char *fmt, ...)
 {
 	va_list args;
 	char buf[NSP32_DEBUG_BUF_LEN];
@@ -697,7 +701,7 @@  static int nsp32_selection_autoscsi(struct scsi_cmnd *SCpnt)
 	nsp32_write1(base, ACK_WIDTH, data->cur_target->ackwidth);
 
 	nsp32_dbg(NSP32_DEBUG_AUTOSCSI,
-		  "syncreg=0x%x, ackwidth=0x%x, sgtpaddr=0x%x, id=0x%x",
+		  "syncreg=0x%x, ackwidth=0x%x, sgtpaddr=0x%lx, id=0x%x",
 		  nsp32_read1(base, SYNC_REG), nsp32_read1(base, ACK_WIDTH),
 		  nsp32_read4(base, SGT_ADR), nsp32_read1(base, SCSI_OUT_LATCH_TARGET_ID));
 	nsp32_dbg(NSP32_DEBUG_AUTOSCSI, "msgout_len=%d, msgout=0x%x",
@@ -888,11 +892,11 @@  static int nsp32_setup_sg_table(struct scsi_cmnd *SCpnt)
 
 			if (le32_to_cpu(sgt[i].len) > 0x10000) {
 				nsp32_msg(KERN_ERR,
-					"can't transfer over 64KB at a time, size=0x%lx", le32_to_cpu(sgt[i].len));
+					"can't transfer over 64KB at a time, size=0x%x", le32_to_cpu(sgt[i].len));
 				return FALSE;
 			}
 			nsp32_dbg(NSP32_DEBUG_SGLIST,
-				  "num 0x%x : addr 0x%lx len 0x%lx",
+				  "num 0x%x : addr 0x%x len 0x%x",
 				  i,
 				  le32_to_cpu(sgt[i].addr),
 				  le32_to_cpu(sgt[i].len ));
@@ -915,7 +919,7 @@  static int nsp32_queuecommand_lck(struct scsi_cmnd *SCpnt, void (*done)(struct s
 
 	nsp32_dbg(NSP32_DEBUG_QUEUECOMMAND,
 		  "enter. target: 0x%x LUN: 0x%llx cmnd: 0x%x cmndlen: 0x%x "
-		  "use_sg: 0x%x reqbuf: 0x%lx reqlen: 0x%x",
+		  "use_sg: 0x%x reqbuf: %p reqlen: 0x%x",
 		  SCpnt->device->id, SCpnt->device->lun, SCpnt->cmnd[0], SCpnt->cmd_len,
 		  scsi_sg_count(SCpnt), scsi_sglist(SCpnt), scsi_bufflen(SCpnt));
 
@@ -2742,7 +2746,7 @@  static int nsp32_detect(struct pci_dev *pdev)
 	res = request_region(host->io_port, host->n_io_port, "nsp32");
 	if (res == NULL) {
 		nsp32_msg(KERN_ERR, 
-			  "I/O region 0x%lx+0x%lx is already used",
+			  "I/O region 0x%x+0x%x is already used",
 			  data->BaseAddress, data->NumAddress);
 		goto free_irq;
         }
@@ -2853,7 +2857,7 @@  static int nsp32_eh_bus_reset(struct scsi_cmnd *SCpnt)
 	spin_lock_irq(SCpnt->device->host->host_lock);
 
 	nsp32_msg(KERN_INFO, "Bus Reset");	
-	nsp32_dbg(NSP32_DEBUG_BUSRESET, "SCpnt=0x%x", SCpnt);
+	nsp32_dbg(NSP32_DEBUG_BUSRESET, "SCpnt=%p", SCpnt);
 
 	nsp32_write2(base, IRQ_CONTROL, IRQ_CONTROL_ALL_IRQ_MASK);
 	nsp32_do_bus_reset(data);
@@ -2912,7 +2916,7 @@  static int nsp32_eh_host_reset(struct scsi_cmnd *SCpnt)
 	nsp32_hw_data    *data = (nsp32_hw_data *)host->hostdata;
 
 	nsp32_msg(KERN_INFO, "Host Reset");	
-	nsp32_dbg(NSP32_DEBUG_BUSRESET, "SCpnt=0x%x", SCpnt);
+	nsp32_dbg(NSP32_DEBUG_BUSRESET, "SCpnt=%p", SCpnt);
 
 	spin_lock_irq(SCpnt->device->host->host_lock);
 
@@ -3307,7 +3311,7 @@  static int nsp32_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	struct Scsi_Host *host = pci_get_drvdata(pdev);
 
-	nsp32_msg(KERN_INFO, "pci-suspend: pdev=0x%p, state=%ld, slot=%s, host=0x%p", pdev, state, pci_name(pdev), host);
+	nsp32_msg(KERN_INFO, "pci-suspend: pdev=0x%p, state=%d, slot=%s, host=0x%p", pdev, state.event, pci_name(pdev), host);
 
 	pci_save_state     (pdev);
 	pci_disable_device (pdev);