[PATCH] [SCSI] osd: fix signed char versus %02x issue

[PATCH] [SCSI] osd: fix signed char versus %02x issue

[Rasmus Villemoes]

If char is signed and one of these bytes happen to have a value
outside the ascii range, the corresponding output will consist of
"ffffff" followed by the two hex chars that were actually
intended. One way to fix it would be to change the casts to (u8*) aka
(unsigned char*), but it is much simpler (and generates smaller code)
to use the %ph extension which was created for such short hexdumps.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
---
 drivers/scsi/osd/osd_initiator.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 0cccd6033feb..d8a2b5185f56 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -170,10 +170,7 @@ static int _osd_get_print_system_info(struct osd_dev *od,
 
 	/* FIXME: Where are the time utilities */
 	pFirst = get_attrs[a++].val_ptr;
-	OSD_INFO("CLOCK                  [0x%02x%02x%02x%02x%02x%02x]\n",
-		((char *)pFirst)[0], ((char *)pFirst)[1],
-		((char *)pFirst)[2], ((char *)pFirst)[3],
-		((char *)pFirst)[4], ((char *)pFirst)[5]);
+	OSD_INFO("CLOCK                  [0x%6phN]\n", pFirst);
 
 	if (a < nelem) { /* IBM-OSD-SIM bug, Might not have it */
 		unsigned len = get_attrs[a].len;
-- 
2.6.1

Re: [PATCH] [SCSI] osd: fix signed char versus %02x issue

[Boaz Harrosh]

On 12/08/2015 04:25 PM, Rasmus Villemoes wrote:
> If char is signed and one of these bytes happen to have a value
> outside the ascii range, the corresponding output will consist of
> "ffffff" followed by the two hex chars that were actually
> intended. One way to fix it would be to change the casts to (u8*) aka
> (unsigned char*), but it is much simpler (and generates smaller code)
> to use the %ph extension which was created for such short hexdumps.
> 

Ha real cool, thanks I hated that crap

ACK-by: Boaz Harrosh <ooo@electrozaur.com>

> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
>  drivers/scsi/osd/osd_initiator.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
> index 0cccd6033feb..d8a2b5185f56 100644
> --- a/drivers/scsi/osd/osd_initiator.c
> +++ b/drivers/scsi/osd/osd_initiator.c
> @@ -170,10 +170,7 @@ static int _osd_get_print_system_info(struct osd_dev *od,
>  
>  	/* FIXME: Where are the time utilities */
>  	pFirst = get_attrs[a++].val_ptr;
> -	OSD_INFO("CLOCK                  [0x%02x%02x%02x%02x%02x%02x]\n",
> -		((char *)pFirst)[0], ((char *)pFirst)[1],
> -		((char *)pFirst)[2], ((char *)pFirst)[3],
> -		((char *)pFirst)[4], ((char *)pFirst)[5]);
> +	OSD_INFO("CLOCK                  [0x%6phN]\n", pFirst);
>  
>  	if (a < nelem) { /* IBM-OSD-SIM bug, Might not have it */
>  		unsigned len = get_attrs[a].len;
> 

Re: [PATCH] [SCSI] osd: fix signed char versus %02x issue

[Martin K. Petersen]

>>>>> "Rasmus" == Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:

Rasmus> If char is signed and one of these bytes happen to have a value
Rasmus> outside the ascii range, the corresponding output will consist
Rasmus> of "ffffff" followed by the two hex chars that were actually
Rasmus> intended. One way to fix it would be to change the casts to
Rasmus> (u8*) aka (unsigned char*), but it is much simpler (and
Rasmus> generates smaller code) to use the %ph extension which was
Rasmus> created for such short hexdumps.

Applied to 4.5/scsi-queue.

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] [SCSI] osd: fix signed char versus %02x issue

[Andy Shevchenko]

On Thu, Dec 10, 2015 at 8:15 PM, Martin K. Petersen
<martin.petersen@oracle.com> wrote:
>>>>>> "Rasmus" == Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:
>
> Rasmus> If char is signed and one of these bytes happen to have a value
> Rasmus> outside the ascii range, the corresponding output will consist
> Rasmus> of "ffffff" followed by the two hex chars that were actually
> Rasmus> intended. One way to fix it would be to change the casts to
> Rasmus> (u8*) aka (unsigned char*), but it is much simpler (and
> Rasmus> generates smaller code) to use the %ph extension which was
> Rasmus> created for such short hexdumps.
>
> Applied to 4.5/scsi-queue.

How fast!

Martin, I have several patches on SCSI subsytem like this one. Some of
them didn't manage kernel (even having Ack!) for years already.
Is it okay if I collect them together and send a bunch once again Cc'ing you?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] [SCSI] osd: fix signed char versus %02x issue

[Martin K. Petersen]

>>>>> "Andy" == Andy Shevchenko <andy.shevchenko@gmail.com> writes:

Andy> I have several patches on SCSI subsytem like this one. Some of
Andy> them didn't manage kernel (even having Ack!) for years already.
Andy> Is it okay if I collect them together and send a bunch once again

Re-sending to linux-scsi is fine. The trick is finding people willing to
review them...

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] [SCSI] osd: fix signed char versus %02x issue

[Martin K. Petersen]

>>>>> "Andy" == Andy Shevchenko <andy.shevchenko@gmail.com> writes:

Andy> I have several patches on SCSI subsytem like this one. Some of
Andy> them didn't manage kernel (even having Ack!) for years already.
Andy> Is it okay if I collect them together and send a bunch once again

Re-sending to linux-scsi is fine. The trick is finding people willing to
review them...

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH] [SCSI] osd: fix signed char versus %02x issue

[Joe Perches]

On Thu, 2015-12-10 at 14:13 -0500, Martin K. Petersen wrote:
> > > > > > "Andy" == Andy Shevchenko <andy.shevchenko@gmail.com> writes:
> 
> Andy> I have several patches on SCSI subsytem like this one. Some of
> Andy> them didn't manage kernel (even having Ack!) for years already.
> Andy> Is it okay if I collect them together and send a bunch once again
> 
> Re-sending to linux-scsi is fine. The trick is finding people willing to
> review them...

Generally speaking, it hasn't been for lack of review.

SCSI has been one of the slowest subsystems to apply
simple defect correction (not whitespace style) patches.

Mostly these patches have been ignored.