[PATCH] bcd: limit bin2bcd input value to lie between 0-99

[PATCH] bcd: limit bin2bcd input value to lie between 0-99

[vbyravarasu]

From: venu byravarasu <vbyravarasu@nvidia.com>

Current implementation of bin2bcd allows any value
between 0x0 to 0xF to be stored in the most significant
nibble of its returned value, against to the BCD limits
of 0 - 9. Hence fixing it.

e.g. say val passed to bin2bcd is 123.
In that case the expected value to be returned by
this function is 0x23.
However, without the fix being added, it would
return 0xC3.

Signed-off-by: venu byravarasu <vbyravarasu@nvidia.com>
---
 lib/bcd.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/bcd.c b/lib/bcd.c
index d74257f..d715474 100644
--- a/lib/bcd.c
+++ b/lib/bcd.c
@@ -9,6 +9,7 @@ EXPORT_SYMBOL(bcd2bin);
 
 unsigned char bin2bcd(unsigned val)
 {
+	val %= 100;
 	return ((val / 10) << 4) + val % 10;
 }
 EXPORT_SYMBOL(bin2bcd);
-- 
1.7.1.1

Re: [PATCH] bcd: limit bin2bcd input value to lie between 0-99

[Andrew Morton]

On Wed, 16 Nov 2011 10:41:32 +0530
vbyravarasu@nvidia.com wrote:

> From: venu byravarasu <vbyravarasu@nvidia.com>
> 
> Current implementation of bin2bcd allows any value
> between 0x0 to 0xF to be stored in the most significant
> nibble of its returned value, against to the BCD limits
> of 0 - 9. Hence fixing it.
> 
> e.g. say val passed to bin2bcd is 123.
> In that case the expected value to be returned by
> this function is 0x23.
> However, without the fix being added, it would
> return 0xC3.
> 

I'm not sure that I agree.  Passing 123 to bin2bcd() is a bug, isn't
it?  We want to know about bugs so we can fix them, and quietly repairing
thngs in callees prevents that.

RE: [PATCH] bcd: limit bin2bcd input value to lie between 0-99

[Venu Byravarasu]

> >
> > e.g. say val passed to bin2bcd is 123.
> > In that case the expected value to be returned by
> > this function is 0x23.
> > However, without the fix being added, it would
> > return 0xC3.
> >
> 
> I'm not sure that I agree.  Passing 123 to bin2bcd() is a bug, isn't
> it?  We want to know about bugs so we can fix them, and quietly repairing
> thngs in callees prevents that.

As bin2bcd is having "unsigned val" as parameter, I do not understand why passing 123 is a bug.
123 that I mentioned here in my example is a decimal value, which is very well within unsigned char limits.
Even any value beyond 99 would create this problem, as the values are not really getting stored as BCD, where each nibble value is to be limited to stay within 0 to 9.
Probably it would be better to change the param of "bin2bcd" from "unsigned val" to "unsigned char" itself.  

RE: [PATCH] bcd: limit bin2bcd input value to lie between 0-99

[Venu Byravarasu]

Hi,

Can someone please approve this change or send comments in case of any more objections?

Thanks,
Venu


> -----Original Message-----
> From: Andrew Morton [mailto:akpm@linux-foundation.org]
> Sent: Wednesday, November 23, 2011 4:02 AM
> To: Venu Byravarasu
> Cc: torvalds@linux-foundation.org; a.zummo@towertech.it; bunk@kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] bcd: limit bin2bcd input value to lie between 0-99
> 
> On Wed, 16 Nov 2011 10:41:32 +0530
> vbyravarasu@nvidia.com wrote:
> 
> > From: venu byravarasu <vbyravarasu@nvidia.com>
> >
> > Current implementation of bin2bcd allows any value
> > between 0x0 to 0xF to be stored in the most significant
> > nibble of its returned value, against to the BCD limits
> > of 0 - 9. Hence fixing it.
> >
> > e.g. say val passed to bin2bcd is 123.
> > In that case the expected value to be returned by
> > this function is 0x23.
> > However, without the fix being added, it would
> > return 0xC3.
> >
> 
> I'm not sure that I agree.  Passing 123 to bin2bcd() is a bug, isn't
> it?  We want to know about bugs so we can fix them, and quietly repairing
> thngs in callees prevents that.

[PATCH] bcd: limit bin2bcd input value to lie between 0-99

[vbyravarasu]

From: venu byravarasu <vbyravarasu@nvidia.com>

If bin2bcd gets a value which is more than 100,
there are chances that it may store non decimal
value in the most significant nibble.

e.g. say val passed to bin2bcd is 112.
In that case the expected value to be returned by
this function is 12.
However, without the fix being added it would
be 112.

Signed-off-by: venu byravarasu <vbyravarasu@nvidia.com>
---
 lib/bcd.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/lib/bcd.c b/lib/bcd.c
index d74257f..d715474 100644
--- a/lib/bcd.c
+++ b/lib/bcd.c
@@ -9,6 +9,7 @@ EXPORT_SYMBOL(bcd2bin);
 
 unsigned char bin2bcd(unsigned val)
 {
+	val %= 100;
 	return ((val / 10) << 4) + val % 10;
 }
 EXPORT_SYMBOL(bin2bcd);
-- 
1.7.1.1