[patch] caif: fix endian conversion in cffrml_transmit()

[patch] caif: fix endian conversion in cffrml_transmit()

[Dan Carpenter]

The "tmp" variable here is used to store the result of cpu_to_le16()
so it should be a u16 instead of an int.  We want the high bits set
and the current code works on little endian systems but not on big
endian systems.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is a static checker thing.  I haven't tested it.  Please review
carefully.

diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index f399211..2b46c48 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -136,10 +136,11 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
 
 static int cffrml_transmit(struct cflayer *layr, struct cfpkt *pkt)
 {
-	int tmp;
+	u16 tmp;
 	u16 chks;
 	u16 len;
 	struct cffrml *this = container_obj(layr);
+
 	if (this->dofcs) {
 		chks = cfpkt_iterate(pkt, cffrml_checksum, 0xffff);
 		tmp = cpu_to_le16(chks);

[patch] caif: fix endian conversion in cffrml_transmit()

[Dan Carpenter]

The "tmp" variable here is used to store the result of cpu_to_le16()
so it should be a u16 instead of an int.  We want the high bits set
and the current code works on little endian systems but not on big
endian systems.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
This is a static checker thing.  I haven't tested it.  Please review
carefully.

diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index f399211..2b46c48 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -136,10 +136,11 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
 
 static int cffrml_transmit(struct cflayer *layr, struct cfpkt *pkt)
 {
-	int tmp;
+	u16 tmp;
 	u16 chks;
 	u16 len;
 	struct cffrml *this = container_obj(layr);
+
 	if (this->dofcs) {
 		chks = cfpkt_iterate(pkt, cffrml_checksum, 0xffff);
 		tmp = cpu_to_le16(chks);

Re: [patch] caif: fix endian conversion in cffrml_transmit()

[Al Viro]

On Mon, Nov 21, 2011 at 09:17:22AM +0300, Dan Carpenter wrote:
> The "tmp" variable here is used to store the result of cpu_to_le16()
> so it should be a u16 instead of an int.  We want the high bits set
> and the current code works on little endian systems but not on big
> endian systems.

Charming...  On b-e we end up feeding zeroes instead of the value we want.

However, that u16 clearly ought to be __le16 - it's there for purpose.

RE: [patch] caif: fix endian conversion in cffrml_transmit()

[Sjur BRENDELAND]

Hi Dan,
 
> The "tmp" variable here is used to store the result of cpu_to_le16()
> so it should be a u16 instead of an int.  We want the high bits set
> and the current code works on little endian systems but not on big
> endian systems.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Looks OK by me, unless you want to make sparse with __CHECK_ENDIAN__ 
shut up, then you should use __le16 ;-)

Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Regards,
Sjur

RE: [patch] caif: fix endian conversion in cffrml_transmit()

[Sjur BRENDELAND]

Hi Dan,
 
> The "tmp" variable here is used to store the result of cpu_to_le16()
> so it should be a u16 instead of an int.  We want the high bits set
> and the current code works on little endian systems but not on big
> endian systems.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Looks OK by me, unless you want to make sparse with __CHECK_ENDIAN__ 
shut up, then you should use __le16 ;-)

Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Regards,
Sjur

--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch v2] caif: fix endian conversion in cffrml_transmit()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

The "tmp" variable here is used to store the result of cpu_to_le16()
so it should be an __le16 instead of an int.  We want the high bits
set and the current code works on little endian systems but not on
big endian systems.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2:  In v1 I used a u16 instead of an __le16.  Also I've changed the
name to "data" instead of "tmp".

diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index f399211..d3ca87b 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -136,20 +136,21 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
 
 static int cffrml_transmit(struct cflayer *layr, struct cfpkt *pkt)
 {
-	int tmp;
 	u16 chks;
 	u16 len;
+	__le16 data;
+
 	struct cffrml *this = container_obj(layr);
 	if (this->dofcs) {
 		chks = cfpkt_iterate(pkt, cffrml_checksum, 0xffff);
-		tmp = cpu_to_le16(chks);
-		cfpkt_add_trail(pkt, &tmp, 2);
+		data = cpu_to_le16(chks);
+		cfpkt_add_trail(pkt, &data, 2);
 	} else {
 		cfpkt_pad_trail(pkt, 2);
 	}
 	len = cfpkt_getlen(pkt);
-	tmp = cpu_to_le16(len);
-	cfpkt_add_head(pkt, &tmp, 2);
+	data = cpu_to_le16(len);
+	cfpkt_add_head(pkt, &data, 2);
 	cfpkt_info(pkt)->hdr_len += 2;
 	if (cfpkt_erroneous(pkt)) {
 		pr_err("Packet is erroneous!\n");

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[patch v2] caif: fix endian conversion in cffrml_transmit()

[Dan Carpenter]

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

The "tmp" variable here is used to store the result of cpu_to_le16()
so it should be an __le16 instead of an int.  We want the high bits
set and the current code works on little endian systems but not on
big endian systems.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
v2:  In v1 I used a u16 instead of an __le16.  Also I've changed the
name to "data" instead of "tmp".

diff --git a/net/caif/cffrml.c b/net/caif/cffrml.c
index f399211..d3ca87b 100644
--- a/net/caif/cffrml.c
+++ b/net/caif/cffrml.c
@@ -136,20 +136,21 @@ static int cffrml_receive(struct cflayer *layr, struct cfpkt *pkt)
 
 static int cffrml_transmit(struct cflayer *layr, struct cfpkt *pkt)
 {
-	int tmp;
 	u16 chks;
 	u16 len;
+	__le16 data;
+
 	struct cffrml *this = container_obj(layr);
 	if (this->dofcs) {
 		chks = cfpkt_iterate(pkt, cffrml_checksum, 0xffff);
-		tmp = cpu_to_le16(chks);
-		cfpkt_add_trail(pkt, &tmp, 2);
+		data = cpu_to_le16(chks);
+		cfpkt_add_trail(pkt, &data, 2);
 	} else {
 		cfpkt_pad_trail(pkt, 2);
 	}
 	len = cfpkt_getlen(pkt);
-	tmp = cpu_to_le16(len);
-	cfpkt_add_head(pkt, &tmp, 2);
+	data = cpu_to_le16(len);
+	cfpkt_add_head(pkt, &data, 2);
 	cfpkt_info(pkt)->hdr_len += 2;
 	if (cfpkt_erroneous(pkt)) {
 		pr_err("Packet is erroneous!\n");

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [patch v2] caif: fix endian conversion in cffrml_transmit()

[Sjur Brændeland]

> The "tmp" variable here is used to store the result of cpu_to_le16()
> so it should be an __le16 instead of an int.  We want the high bits
> set and the current code works on little endian systems but not on
> big endian systems.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2:  In v1 I used a u16 instead of an __le16.  Also I've changed the
> name to "data" instead of "tmp".

Thanks Dan, good catch - this would have failed badly on a b_e architecture.

Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Regards,
Sjur

Re: [patch v2] caif: fix endian conversion in cffrml_transmit()

[Sjur Brændeland]

> The "tmp" variable here is used to store the result of cpu_to_le16()
> so it should be an __le16 instead of an int.  We want the high bits
> set and the current code works on little endian systems but not on
> big endian systems.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> v2:  In v1 I used a u16 instead of an __le16.  Also I've changed the
> name to "data" instead of "tmp".

Thanks Dan, good catch - this would have failed badly on a b_e architecture.

Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Regards,
Sjur

Re: [patch v2] caif: fix endian conversion in cffrml_transmit()

[David Miller]

From: Sjur Brændeland <sjurbren@gmail.com>
Date: Mon, 21 Nov 2011 22:20:09 +0100

>> The "tmp" variable here is used to store the result of cpu_to_le16()
>> so it should be an __le16 instead of an int.  We want the high bits
>> set and the current code works on little endian systems but not on
>> big endian systems.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> v2:  In v1 I used a u16 instead of an __le16.  Also I've changed the
>> name to "data" instead of "tmp".
> 
> Thanks Dan, good catch - this would have failed badly on a b_e architecture.
> 
> Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied, thanks guys.

Re: [patch v2] caif: fix endian conversion in cffrml_transmit()

[David Miller]

From: Sjur Brændeland <sjurbren@gmail.com>
Date: Mon, 21 Nov 2011 22:20:09 +0100

>> The "tmp" variable here is used to store the result of cpu_to_le16()
>> so it should be an __le16 instead of an int.  We want the high bits
>> set and the current code works on little endian systems but not on
>> big endian systems.
>>
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> ---
>> v2:  In v1 I used a u16 instead of an __le16.  Also I've changed the
>> name to "data" instead of "tmp".
> 
> Thanks Dan, good catch - this would have failed badly on a b_e architecture.
> 
> Acked-by: Sjur Brændeland <sjur.brandeland@stericsson.com>

Applied, thanks guys.
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html