headers
Finn Thain | 1 Jan 2011 07:48
Picon
Favicon

Re: [PATCH 03/15]drivers:staging:rtl8187se:r8180_hw.h Typo change diable to disable.


On Thu, 30 Dec 2010, Justin P. Mattock wrote:

> The below patch fixes a typo "diable" to "disable". Please let me know if this 
> is correct or not.
> 
> Signed-off-by: Justin P. Mattock <justinmattock <at> gmail.com>
> 
> ---
>  drivers/staging/rtl8187se/r8180_hw.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/staging/rtl8187se/r8180_hw.h b/drivers/staging/rtl8187se/r8180_hw.h
> index 3fca144..2911d40 100644
> --- a/drivers/staging/rtl8187se/r8180_hw.h
> +++ b/drivers/staging/rtl8187se/r8180_hw.h
>  <at>  <at>  -554,7 +554,7  <at>  <at> 
>  /* by amy for power save		*/
>  /* by amy for antenna			*/
>  #define EEPROM_SW_REVD_OFFSET 0x3f
> -/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other values
are diable.					*/
> +/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other values
are disabled.					*/

I think, "other values disable" was what you meant?

Finn

>  #define EEPROM_SW_AD_MASK			0x0300
(Continue reading)

Permalink | Reply | 11 comments |
headers
Justin P. Mattock | 1 Jan 2011 08:43
Picon

Re: [PATCH 03/15]drivers:staging:rtl8187se:r8180_hw.h Typo change diable to disable.

On 12/31/2010 10:48 PM, Finn Thain wrote:
>
> On Thu, 30 Dec 2010, Justin P. Mattock wrote:
>
>> The below patch fixes a typo "diable" to "disable". Please let me know if this
>> is correct or not.
>>
>> Signed-off-by: Justin P. Mattock<justinmattock <at> gmail.com>
>>
>> ---
>>   drivers/staging/rtl8187se/r8180_hw.h |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8187se/r8180_hw.h b/drivers/staging/rtl8187se/r8180_hw.h
>> index 3fca144..2911d40 100644
>> --- a/drivers/staging/rtl8187se/r8180_hw.h
>> +++ b/drivers/staging/rtl8187se/r8180_hw.h
>>  <at>  <at>  -554,7 +554,7  <at>  <at> 
>>   /* by amy for power save		*/
>>   /* by amy for antenna			*/
>>   #define EEPROM_SW_REVD_OFFSET 0x3f
>> -/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other
values are diable.					*/
>> +/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other
values are disabled.					*/
>
> I think, "other values disable" was what you meant?
>
> Finn
>
(Continue reading)

Permalink | Reply | 9 comments |
headers
Dan Carpenter | 1 Jan 2011 10:00
Picon

Re: [PATCH 1/2] staging: keucr: Use memcmp() instaed of custom StringCmp() and some style cleanups

On Fri, Dec 31, 2010 at 12:26:00PM -0800, Joe Perches wrote:
> On Fri, 2010-12-31 at 20:28 +0100, Javier Martinez Canillas wrote:
> 	if (!(!memcmp(redundant+0x0D, EccBuf, 3) &&
> 	      !memcmp(redundant+0x08, EccBuf+0x03, 3)))
                              ^^^          ^^^

Also we should add spaces around the '+' character.  But this is dead
code anyway so let's just merge it.

regards,
dan carpenter
Permalink | Reply | 0 comments |
headers
Dan Carpenter | 1 Jan 2011 10:09
Picon

Re: [PATCH 03/15]drivers:staging:rtl8187se:r8180_hw.h Typo change diable to disable.

On Fri, Dec 31, 2010 at 11:43:30PM -0800, Justin P. Mattock wrote:
> On 12/31/2010 10:48 PM, Finn Thain wrote:
> >>-/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other
values are diable.					*/
> >>+/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other
values are disabled.					*/
> >
> >I think, "other values disable" was what you meant?
> >
> >Finn
> >
> >>  #define EEPROM_SW_AD_MASK			0x0300
> >>  #define EEPROM_SW_AD_ENABLE			0x0100
> >>
> >>
> >
> 
> no! I changed it to disabled to make it proper..

Finn is obviously right, but maybe a compromise would be:

Only the value EEPROM_SW_AD_ENABLE means "enable", other values mean
"disable".

regards,
dan carpenter
Permalink | Reply | 7 comments |
headers
Justin P. Mattock | 1 Jan 2011 15:53
Picon

Re: [PATCH 03/15]drivers:staging:rtl8187se:r8180_hw.h Typo change diable to disable.

On 01/01/2011 01:09 AM, Dan Carpenter wrote:
> On Fri, Dec 31, 2010 at 11:43:30PM -0800, Justin P. Mattock wrote:
>> On 12/31/2010 10:48 PM, Finn Thain wrote:
>>>> -/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other
values are diable.					*/
>>>> +/*  BIT[8-9] is for SW Antenna Diversity. Only the value EEPROM_SW_AD_ENABLE means enable, other
values are disabled.					*/
>>>
>>> I think, "other values disable" was what you meant?
>>>
>>> Finn
>>>
>>>>   #define EEPROM_SW_AD_MASK			0x0300
>>>>   #define EEPROM_SW_AD_ENABLE			0x0100
>>>>
>>>>
>>>
>>
>> no! I changed it to disabled to make it proper..
>
> Finn is obviously right, but maybe a compromise would be:
>
> Only the value EEPROM_SW_AD_ENABLE means "enable", other values mean
> "disable".
>
> regards,
> dan carpenter
>

ahh.. I see what you your saying now.. alright let me send this out that
(Continue reading)

Permalink | Reply | 0 comments |
headers
Gábor Stefanik | 2 Jan 2011 00:41
Picon

Re: [PATCH 2/2] staging: keucr: Use memcpy() instad of custom StringCopy() and some style cleanups

On Fri, Dec 31, 2010 at 8:28 PM, Javier Martinez Canillas
<martinez.javier <at> gmail.com> wrote:
>
> Signed-off-by: Javier Martinez Canillas <martinez.javier <at> gmail.com>
> ---
>  drivers/staging/keucr/smilecc.c |   16 +++++++++-------
>  1 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/keucr/smilecc.c b/drivers/staging/keucr/smilecc.c
> index daf322a..f9e515d 100644
> --- a/drivers/staging/keucr/smilecc.c
> +++ b/drivers/staging/keucr/smilecc.c
>  <at>  <at>  -182,13 +182,15  <at>  <at>  BYTE *buf;
>  BYTE *redundant_ecc;
>  BYTE *calculate_ecc;
>  {
> -    DWORD err;
> -
> -    err=correct_data(buf,redundant_ecc,*(calculate_ecc+1),*(calculate_ecc),*(calculate_ecc+2));
> -    if (err==1) StringCopy(calculate_ecc,redundant_ecc,3);
> -        if (err==0 || err==1 || err==2)
> -            return(0);
> -    return(-1);
> +       DWORD err;
> +       err = correct_data(buf, redundant_ecc, *(calculate_ecc+1),
> +                          *(calculate_ecc), *(calculate_ecc+2));

Any reason why you didn't unify these 2 lines? Like this: DWORD err =
correct_data(...);
(Continue reading)

Permalink | Reply | 1 comment |
headers
Dan Carpenter | 2 Jan 2011 06:34
Picon

Re: [PATCH 2/2] staging: keucr: Use memcpy() instad of custom StringCopy() and some style cleanups

On Sun, Jan 02, 2011 at 12:41:33AM +0100, Gábor Stefanik wrote:
> On Fri, Dec 31, 2010 at 8:28 PM, Javier Martinez Canillas
> <martinez.javier <at> gmail.com> wrote:
> > +       DWORD err;
> > +       err = correct_data(buf, redundant_ecc, *(calculate_ecc+1),
> > +                          *(calculate_ecc), *(calculate_ecc+2));
> 
> Any reason why you didn't unify these 2 lines? Like this: DWORD err =
> correct_data(...);
> 

These kind of things aren't described in CodingStyle so they're up to
whoever writes the code to decide.  Or if the maintainer is a
micromanager the maintainer can decide.

But personally I much prefer to put anything complicated on separate
lines.  No one reads the initializers.  In my work with Smatch I see a
lot of bugs like this:

	int x = foo->bar;

	if (!foo)
		return -EINVAL;

It's astounding how many.  The famous tun.c security bug was one of
these.

But there should  have been a blank line between the initializers and
the code.  Otherwise people will think the code is initiliazation and
ignore it.  That is in CodingStyle I think.  We can fix that when we get
(Continue reading)

Permalink | Reply | 0 comments |
headers
Javier Martinez Canillas | 2 Jan 2011 22:01
Picon
Gravatar

[PATCHv3 0/4] staging: keucr: Use memcmp() & memcpy() instead of custom String[Cmp|Copy] functions

This patchset make some cleanup to staging/keucr. Use memcmp and memcpy instead of custom functions.
Also some style cleanups recomended by Dan Carpenter, Joe Perches and Gábor Stefanik.

The patches in this patchset are the following:

[PATCH 1/3] staging: keucr: Use memcmp() instead custom StringCmp() and some style cleanups
[PATCH 2/3] staging: keucr: Use memcpy() instead custom StringCopy() and some style cleanups
[PATCH 3/3] staging: keucr: Delete StringCmp() and StringCopy custom functions
[PATCH 4/4] staging: keucr: Delete use kernel strcmp() & strcpy() from TODO file
_______________________________________________
devel mailing list
devel <at> linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
Permalink | Reply | 9 comments |
headers
Javier Martinez Canillas | 2 Jan 2011 22:01
Picon
Gravatar

[PATCH 1/3] staging: keucr: Use memcmp() instead custom StringCmp() and some style cleanups


Signed-off-by: Javier Martinez Canillas <martinez.javier <at> gmail.com>
---
 drivers/staging/keucr/smilsub.c |   59 ++++++++++++++++++++-------------------
 1 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/keucr/smilsub.c b/drivers/staging/keucr/smilsub.c
index ce10cf2..a15cc02 100644
--- a/drivers/staging/keucr/smilsub.c
+++ b/drivers/staging/keucr/smilsub.c
 <at>  <at>  -1482,54 +1482,55  <at>  <at>  BYTE _Check_D_DevCode(BYTE dcode)
 //----- Check_D_ReadError() ----------------------------------------------
 int Check_D_ReadError(BYTE *redundant)
 {
-    // Driver ¤£°µ ECC Check
-    return(SUCCESS);
-    if (!StringCmp((char *)(redundant+0x0D),(char *)EccBuf,3))
-        if (!StringCmp((char *)(redundant+0x08),(char *)(EccBuf+0x03),3))
-            return(SUCCESS);
+	/* Driver ECC Check */
+	if (memcmp(redundant + 0x0D, EccBuf, 3) ||
+	    memcmp(redundant + 0x08, EccBuf + 0x03, 3))
+		return ERROR;

-    return(ERROR);
+	return SUCCESS;
 }

 //----- Check_D_Correct() ----------------------------------------------
 int Check_D_Correct(BYTE *buf,BYTE *redundant)
(Continue reading)

Permalink | Reply | 3 comments |
headers
Javier Martinez Canillas | 2 Jan 2011 22:01
Picon
Gravatar

[PATCH 3/3] staging: keucr: Delete StringCmp() and StringCopy custom functions


Signed-off-by: Javier Martinez Canillas <martinez.javier <at> gmail.com>
---
 drivers/staging/keucr/smilsub.c |   19 -------------------
 1 files changed, 0 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/keucr/smilsub.c b/drivers/staging/keucr/smilsub.c
index a15cc02..326a04c 100644
--- a/drivers/staging/keucr/smilsub.c
+++ b/drivers/staging/keucr/smilsub.c
 <at>  <at>  -1590,25 +1590,6  <at>  <at>  char Bit_D_CountWord(WORD cdata)
     return((char)bitcount);
 }

-void StringCopy(char *stringA, char *stringB, int count)
-{
-    int i;
-
-    for(i=0; i<count; i++)
-        *stringA++ = *stringB++;
-}
-
-//-----
-int StringCmp(char *stringA, char *stringB, int count)
-{
-    int i;
-
-    for (i=0;i<count;i++)
-        if (*stringA++ != *stringB++)
-            return(ERROR);
(Continue reading)

Permalink | Reply | 0 comments |

Gmane