| Message ID | 20150422231618.00005ec8@localhost (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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,
Remove trailing space from model description. Signed-off-by: Sebastian Herbszt <herbszt@gmx.de> -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html