Re: [PATCH] lpfc: fix model description

[Sebastian Herbszt <herbszt@gmx.de>]

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