lpfc: fix model description

PATCH Superseded Sebastian Herbszt 3 participants 6 messages Apr 22, 2015 – May 28, 2015

Subsystems (2)

SCSI Emulex/Broadcom lpfc FC/FCoE SCSI driver

lore

Share & tools

lore

Sebastian Herbszt

author

2015-04-22 21:16 UTC #0 lore raw

PATCH 1/1 lpfc: fix model description

Remove trailing space from model description.

Signed-off-by Sebastian Herbszt <[email protected]>

drivers/scsi/lpfc/lpfc_init.c renamed from drivers/scsi/lpfc.orig/lpfc_init.c +1 -1

diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c
@@ -2253,7 +2253,7 @@ lpfc_get_hba_model_desc(struct lpfc_hba
 				phba->Port);
 		else if (max_speed == 0)
 			snprintf(descp, 255,
-				"Emulex %s %s %s ",
+				"Emulex %s %s %s",
 				m.name, m.bus, m.function);
 		else
 			snprintf(descp, 255,

Since you are touching that code, please include the following additional changes in your patch or in a separate patch:
* Change the type of the third argument of lpfc_get_hba_model_desc()
from uint8_t * into char * since that pointer is used to store a
'\0'-terminated ASCII string.
* Add a size argument to lpfc_get_hba_model_desc() such that the callers
of this function can specify what the size is of the array descp
points at (sizeof(phba->ModelDesc) ?).
* Switch from snprintf() to scnprintf() because the latter function
guarantees '\0'-termination if the output has to be truncated.

Thanks,

Bart.

Sebastian Herbszt

author

2015-04-26 22:32 UTC #2 lore raw

Quoted 5 lines · Bart Van Assche

Quoted 4 lines · Sebastian Herbszt
Remove trailing space from model description.
Signed-off-by Sebastian Herbszt <[email protected]>

Quoted 14 lines

drivers/scsi/lpfc/lpfc_init.c lpfc_get_hba_model_desc(struct lpfc_hba · line 2259
Quoted 12 lines
diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c
drivers/scsi/lpfc/lpfc_init.c renamed from drivers/scsi/lpfc.orig/lpfc_init.c +1 -1
@@ -2253,7 +2253,7 @@ lpfc_get_hba_model_desc(struct lpfc_hba
  				phba->Port);
  		else if (max_speed == 0)
  			snprintf(descp, 255,
-				"Emulex %s %s %s ",
+				"Emulex %s %s %s",
  				m.name, m.bus, m.function);
  		else
  			snprintf(descp, 255,
Hello Sebastian,

Hello Bart,

Quoted 2 lines

Since you are touching that code, please include the following additional changes in your patch or in a separate patch:

I could follow up with a patch containing

Quoted 3 lines

* Change the type of the third argument of lpfc_get_hba_model_desc() from uint8_t * into char * since that pointer is used to store a '\0'-terminated ASCII string.

-static void lpfc_get_hba_model_desc(struct lpfc_hba *, uint8_t *, uint8_t *); +static void lpfc_get_hba_model_desc(struct lpfc_hba *, char *, char *);

and

-lpfc_get_hba_model_desc(struct lpfc_hba *phba, uint8_t *mdp, uint8_t *descp) +lpfc_get_hba_model_desc(struct lpfc_hba *phba, char *mdp, char *descp)

since both mdp and descp are strings.

Quoted 3 lines

* Add a size argument to lpfc_get_hba_model_desc() such that the callers of this function can specify what the size is of the array descp points at (sizeof(phba->ModelDesc) ?).

I am not sure whether this change is necessary because callers of lpfc_get_hba_model_desc() are limited to lpfc_init.c and always use the same arguments (phba->ModelName, phba->ModelDesc):

char ModelDesc[256]; /* Model Description */
char ModelName[80]; /* Model Name */

Quoted 2 lines

* Switch from snprintf() to scnprintf() because the latter function guarantees '\0'-termination if the output has to be truncated.

Doesn't snprintf() guarantee this too? The only difference between snprintf() and scnprintf() seems to be the return value. snprintf() calls vsnprintf() and scnprintf() calls vscnprintf() which calls vsnprintf() itself.

Quoted 4 lines

Thanks,
Bart.

Sebastian

Bart Van Assche

2015-04-27 06:03 UTC #3 lore raw

Quoted 8 lines · Sebastian Herbszt

Quoted 2 lines · Bart Van Assche
* Switch from snprintf() to scnprintf() because the latter function guarantees '\0'-termination if the output has to be truncated.
Doesn't snprintf() guarantee this too? The only difference between snprintf() and scnprintf() seems to be the return value. snprintf() calls vsnprintf() and scnprintf() calls vscnprintf() which calls vsnprintf() itself.

Hello Sebastian,

In my e-mail I was referring to the pre-C99 behavior of snprintf(). Apparently the snprintf() function in the Linux kernel is compliant with the C99 specs. See e.g. http://pubs.opengroup.org/onlinepubs/7908799/xsh/fprintf.html and http://pubs.opengroup.org/stage7tc1/functions/fprintf.html#. In the latter document it has been mentioned that "output bytes beyond the n-1st shall be discarded", something which has not been mentioned in the former document.

Bart.

Sebastian Herbszt

author

2015-05-27 21:40 UTC #4 lore raw

In reply to Sebastian Herbszt

Quoted 8 lines · I wrote:

Remove trailing space from model description.
Signed-off-by Sebastian Herbszt <[email protected]>
diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c

drivers/scsi/lpfc/lpfc_init.c lpfc_get_hba_model_desc(struct lpfc_hba · line 2259

Quoted 8 lines

drivers/scsi/lpfc/lpfc_init.c renamed from drivers/scsi/lpfc.orig/lpfc_init.c +1 -1

@@ -2253,7 +2253,7 @@ lpfc_get_hba_model_desc(struct lpfc_hba
 				phba->Port);
 		else if (max_speed == 0)
 			snprintf(descp, 255,
-				"Emulex %s %s %s ",
+				"Emulex %s %s %s",
 				m.name, m.bus, m.function);
 		else
 			snprintf(descp, 255,

James,

any feedback?

Sebastian

James Smart

Reviewed-by

2015-05-28 20:28 UTC #5 lore raw

Yes - this is acceptable. Not a big deal per-say, but good to be uniform.

Reviewed-by James Smart <[email protected]>

-- james s

Quoted 22 lines · Sebastian Herbszt

Quoted 8 lines · I wrote:
Remove trailing space from model description.
Signed-off-by Sebastian Herbszt <[email protected]>
diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c
drivers/scsi/lpfc/lpfc_init.c lpfc_get_hba_model_desc(struct lpfc_hba · line 2259
Quoted 8 lines
drivers/scsi/lpfc/lpfc_init.c renamed from drivers/scsi/lpfc.orig/lpfc_init.c +1 -1
@@ -2253,7 +2253,7 @@ lpfc_get_hba_model_desc(struct lpfc_hba
  				phba->Port);
  		else if (max_speed == 0)
  			snprintf(descp, 255,
-				"Emulex %s %s %s ",
+				"Emulex %s %s %s",
  				m.name, m.bus, m.function);
  		else
  			snprintf(descp, 255,
James,
any feedback?
Sebastian