All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] lpfc: fix model description
@ 2015-04-22 21:16 Sebastian Herbszt
  2015-04-23  7:36 ` Bart Van Assche
  2015-05-27 21:40 ` Sebastian Herbszt
  0 siblings, 2 replies; 6+ messages in thread
From: Sebastian Herbszt @ 2015-04-22 21:16 UTC (permalink / raw)
  To: linux-scsi; +Cc: James Smart, Sebastian Herbszt

Remove trailing space from model description.

Signed-off-by: Sebastian Herbszt <herbszt@gmx.de>

diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c
--- 4.0/drivers/scsi/lpfc.orig/lpfc_init.c	2015-04-15 06:18:24.673045138 +0200
+++ 4.0/drivers/scsi/lpfc/lpfc_init.c	2015-04-22 21:03:39.203230409 +0200
@@ -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,

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lpfc: fix model description
  2015-04-22 21:16 [PATCH] lpfc: fix model description Sebastian Herbszt
@ 2015-04-23  7:36 ` Bart Van Assche
  2015-04-26 22:32   ` Sebastian Herbszt
  2015-05-27 21:40 ` Sebastian Herbszt
  1 sibling, 1 reply; 6+ messages in thread
From: Bart Van Assche @ 2015-04-23  7:36 UTC (permalink / raw)
  To: Sebastian Herbszt, linux-scsi; +Cc: James Smart

On 04/22/15 23:16, Sebastian Herbszt wrote:
> Remove trailing space from model description.
>
> Signed-off-by: Sebastian Herbszt <herbszt@gmx.de>
>
> diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c
> --- 4.0/drivers/scsi/lpfc.orig/lpfc_init.c	2015-04-15 06:18:24.673045138 +0200
> +++ 4.0/drivers/scsi/lpfc/lpfc_init.c	2015-04-22 21:03:39.203230409 +0200
> @@ -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,

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.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lpfc: fix model description
  2015-04-23  7:36 ` Bart Van Assche
@ 2015-04-26 22:32   ` Sebastian Herbszt
  2015-04-27  6:02     ` Bart Van Assche
  0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Herbszt @ 2015-04-26 22:32 UTC (permalink / raw)
  To: Bart Van Assche; +Cc: linux-scsi, James Smart, Sebastian Herbszt

Bart Van Assche wrote:
> On 04/22/15 23:16, Sebastian Herbszt wrote:
> > Remove trailing space from model description.
> >
> > Signed-off-by: Sebastian Herbszt <herbszt@gmx.de>
> >
> > diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c
> > --- 4.0/drivers/scsi/lpfc.orig/lpfc_init.c	2015-04-15 06:18:24.673045138 +0200
> > +++ 4.0/drivers/scsi/lpfc/lpfc_init.c	2015-04-22 21:03:39.203230409 +0200
> > @@ -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,

> 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

> * 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.

> * 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 */

> * 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.

> Thanks,
> 
> Bart.
> 

Sebastian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lpfc: fix model description
  2015-04-26 22:32   ` Sebastian Herbszt
@ 2015-04-27  6:02     ` Bart Van Assche
  0 siblings, 0 replies; 6+ messages in thread
From: Bart Van Assche @ 2015-04-27  6:02 UTC (permalink / raw)
  To: Sebastian Herbszt; +Cc: linux-scsi, James Smart

On 04/27/15 00:32, Sebastian Herbszt wrote:
> Bart Van Assche wrote:
>> * 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lpfc: fix model description
  2015-04-22 21:16 [PATCH] lpfc: fix model description Sebastian Herbszt
  2015-04-23  7:36 ` Bart Van Assche
@ 2015-05-27 21:40 ` Sebastian Herbszt
  2015-05-28 20:28   ` James Smart
  1 sibling, 1 reply; 6+ messages in thread
From: Sebastian Herbszt @ 2015-05-27 21:40 UTC (permalink / raw)
  To: James Smart; +Cc: linux-scsi, Sebastian Herbszt

I wrote:
> Remove trailing space from model description.
> 
> Signed-off-by: Sebastian Herbszt <herbszt@gmx.de>
> 
> diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c
> --- 4.0/drivers/scsi/lpfc.orig/lpfc_init.c	2015-04-15 06:18:24.673045138 +0200
> +++ 4.0/drivers/scsi/lpfc/lpfc_init.c	2015-04-22 21:03:39.203230409 +0200
> @@ -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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lpfc: fix model description
  2015-05-27 21:40 ` Sebastian Herbszt
@ 2015-05-28 20:28   ` James Smart
  0 siblings, 0 replies; 6+ messages in thread
From: James Smart @ 2015-05-28 20:28 UTC (permalink / raw)
  To: Sebastian Herbszt; +Cc: linux-scsi

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

Reviewed-By: James Smart <james.smart@avagotech.com>

-- james s


On 5/27/2015 5:40 PM, Sebastian Herbszt wrote:
> I wrote:
>> Remove trailing space from model description.
>>
>> Signed-off-by: Sebastian Herbszt <herbszt@gmx.de>
>>
>> diff -up 4.0/drivers/scsi/lpfc.orig/lpfc_init.c 4.0/drivers/scsi/lpfc/lpfc_init.c
>> --- 4.0/drivers/scsi/lpfc.orig/lpfc_init.c	2015-04-15 06:18:24.673045138 +0200
>> +++ 4.0/drivers/scsi/lpfc/lpfc_init.c	2015-04-22 21:03:39.203230409 +0200
>> @@ -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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2015-05-28 20:28 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-22 21:16 [PATCH] lpfc: fix model description Sebastian Herbszt
2015-04-23  7:36 ` Bart Van Assche
2015-04-26 22:32   ` Sebastian Herbszt
2015-04-27  6:02     ` Bart Van Assche
2015-05-27 21:40 ` Sebastian Herbszt
2015-05-28 20:28   ` James Smart

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.