Skip to contentSkip to conversation

Crypto: gf128mul : fixed a parentheses coding style issue

PATCH In review Mike Roocroft 2 participants 6 messages Oct 12, 2014 – Oct 13, 2014 Crypto API

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Crypto: gf128mul : fixed a parentheses coding style issue

Fixed a coding style issue.

---
 crypto/gf128mul.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
crypto/gf128mul.c +1 -1
index 5276607..b143d84 100644
@@ -97,7 +97,7 @@
     the table above
 */
 
-#define xx(p, q)	0x##p##q
+#define xx(p, q)	(0x##p##q)
 
 #define xda_bbe(i) ( \
 	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \
Signature · 1 line
2.1.0
#1 lore raw
Quoted 1 line · Mike Roocroft

Fixed a coding style issue.

[]

crypto/gf128mul.c

Quoted 1 line
crypto/gf128mul.c +0 -0

[]

crypto/gf128mul.c line 103

Quoted 9 lines
crypto/gf128mul.c +1 -1
@@ -97,7 +97,7 @@
     the table above
 */
 
-#define xx(p, q)	0x##p##q
+#define xx(p, q)	(0x##p##q)
 
 #define xda_bbe(i) ( \
 	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \

I think that macro is pretty useless and nothing
but obfuscation now.

The comment above it doesn't seem useful either.

How about just removing and replacing the uses
like this?

---
 crypto/gf128mul.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)
crypto/gf128mul.c +8 -19
index 5276607..90cf17d 100644
@@ -88,29 +88,18 @@
 	q(0xf8), q(0xf9), q(0xfa), q(0xfb), q(0xfc), q(0xfd), q(0xfe), q(0xff) \
 }
 
-/*	Given the value i in 0..255 as the byte overflow when a field element
-    in GHASH is multiplied by x^8, this function will return the values that
-    are generated in the lo 16-bit word of the field value by applying the
-    modular polynomial. The values lo_byte and hi_byte are returned via the
-    macro xp_fun(lo_byte, hi_byte) so that the values can be assembled into
-    memory as required by a suitable definition of this macro operating on
-    the table above
-*/
-
-#define xx(p, q)	0x##p##q
-
 #define xda_bbe(i) ( \
-	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \
-	(i & 0x20 ? xx(10, e0) : 0) ^ (i & 0x10 ? xx(08, 70) : 0) ^ \
-	(i & 0x08 ? xx(04, 38) : 0) ^ (i & 0x04 ? xx(02, 1c) : 0) ^ \
-	(i & 0x02 ? xx(01, 0e) : 0) ^ (i & 0x01 ? xx(00, 87) : 0) \
+	(i & 0x80 ? 0x4380 : 0) ^ (i & 0x40 ? 0x21c0 : 0) ^ \
+	(i & 0x20 ? 0x10e0 : 0) ^ (i & 0x10 ? 0x0870 : 0) ^ \
+	(i & 0x08 ? 0x0438 : 0) ^ (i & 0x04 ? 0x021c : 0) ^ \
+	(i & 0x02 ? 0x010e : 0) ^ (i & 0x01 ? 0x0087 : 0) \
 )
 
 #define xda_lle(i) ( \
-	(i & 0x80 ? xx(e1, 00) : 0) ^ (i & 0x40 ? xx(70, 80) : 0) ^ \
-	(i & 0x20 ? xx(38, 40) : 0) ^ (i & 0x10 ? xx(1c, 20) : 0) ^ \
-	(i & 0x08 ? xx(0e, 10) : 0) ^ (i & 0x04 ? xx(07, 08) : 0) ^ \
-	(i & 0x02 ? xx(03, 84) : 0) ^ (i & 0x01 ? xx(01, c2) : 0) \
+	(i & 0x80 ? 0xe100 : 0) ^ (i & 0x40 ? 0x7080 : 0) ^ \
+	(i & 0x20 ? 0x3840 : 0) ^ (i & 0x10 ? 0x1c20 : 0) ^ \
+	(i & 0x08 ? 0x0e10 : 0) ^ (i & 0x04 ? 0x0708 : 0) ^ \
+	(i & 0x02 ? 0x0384 : 0) ^ (i & 0x01 ? 0x01c2 : 0) \
 )
 
 static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);
#2 lore raw
Quoted 61 lines · Joe Perches
Quoted 1 line · Mike Roocroft

Fixed a coding style issue.

[]

crypto/gf128mul.c

Quoted 1 line
crypto/gf128mul.c +0 -0

[]

crypto/gf128mul.c line 103

Quoted 9 lines
crypto/gf128mul.c +1 -1
@@ -97,7 +97,7 @@
      the table above
  */
 
-#define xx(p, q)	0x##p##q
+#define xx(p, q)	(0x##p##q)
 
  #define xda_bbe(i) ( \
  	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \

I think that macro is pretty useless and nothing
but obfuscation now.

The comment above it doesn't seem useful either.

How about just removing and replacing the uses
like this?

---
  crypto/gf128mul.c | 27 ++++++++-------------------
  1 file changed, 8 insertions(+), 19 deletions(-)
crypto/gf128mul.c +4 -16
index 5276607..90cf17d 100644
@@ -88,29 +88,18 @@
  	q(0xf8), q(0xf9), q(0xfa), q(0xfb), q(0xfc), q(0xfd), q(0xfe), q(0xff) \
  }
 
-/*	Given the value i in 0..255 as the byte overflow when a field element
-    in GHASH is multiplied by x^8, this function will return the values that
-    are generated in the lo 16-bit word of the field value by applying the
-    modular polynomial. The values lo_byte and hi_byte are returned via the
-    macro xp_fun(lo_byte, hi_byte) so that the values can be assembled into
-    memory as required by a suitable definition of this macro operating on
-    the table above
-*/
-
-#define xx(p, q)	0x##p##q
-
  #define xda_bbe(i) ( \
-	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \
-	(i & 0x20 ? xx(10, e0) : 0) ^ (i & 0x10 ? xx(08, 70) : 0) ^ \
-	(i & 0x08 ? xx(04, 38) : 0) ^ (i & 0x04 ? xx(02, 1c) : 0) ^ \
-	(i & 0x02 ? xx(01, 0e) : 0) ^ (i & 0x01 ? xx(00, 87) : 0) \
+	(i & 0x80 ? 0x4380 : 0) ^ (i & 0x40 ? 0x21c0 : 0) ^ \
+	(i & 0x20 ? 0x10e0 : 0) ^ (i & 0x10 ? 0x0870 : 0) ^ \
+	(i & 0x08 ? 0x0438 : 0) ^ (i & 0x04 ? 0x021c : 0) ^ \
+	(i & 0x02 ? 0x010e : 0) ^ (i & 0x01 ? 0x0087 : 0) \
  )
 
  #define xda_lle(i) ( \
-	(i & 0x80 ? xx(e1, 00) : 0) ^ (i & 0x40 ? xx(70, 80) : 0) ^ \

crypto/gf128mul.c line 105

Quoted 10 lines
crypto/gf128mul.c +4 -3
-	(i & 0x20 ? xx(38, 40) : 0) ^ (i & 0x10 ? xx(1c, 20) : 0) ^ \
-	(i & 0x08 ? xx(0e, 10) : 0) ^ (i & 0x04 ? xx(07, 08) : 0) ^ \
-	(i & 0x02 ? xx(03, 84) : 0) ^ (i & 0x01 ? xx(01, c2) : 0) \
+	(i & 0x80 ? 0xe100 : 0) ^ (i & 0x40 ? 0x7080 : 0) ^ \
+	(i & 0x20 ? 0x3840 : 0) ^ (i & 0x10 ? 0x1c20 : 0) ^ \
+	(i & 0x08 ? 0x0e10 : 0) ^ (i & 0x04 ? 0x0708 : 0) ^ \
+	(i & 0x02 ? 0x0384 : 0) ^ (i & 0x01 ? 0x01c2 : 0) \
  )
 
  static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);

Hi there,

I'm not really contributing anything other than getting code checkpatch clean, whilst

I relearn C and get a feel for various parts of the kernel.

#3 lore raw
Quoted 68 lines · Michael Roocroft
Quoted 67 lines · Joe Perches
Quoted 1 line · Mike Roocroft

Fixed a coding style issue.

[]

crypto/gf128mul.c

Quoted 1 line
crypto/gf128mul.c +0 -0

[]

crypto/gf128mul.c line 103

Quoted 9 lines
crypto/gf128mul.c +1 -1
@@ -97,7 +97,7 @@
      the table above
  */
 
-#define xx(p, q)	0x##p##q
+#define xx(p, q)	(0x##p##q)
 
  #define xda_bbe(i) ( \
  	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \

I think that macro is pretty useless and nothing
but obfuscation now.

The comment above it doesn't seem useful either.

How about just removing and replacing the uses
like this?

---
  crypto/gf128mul.c | 27 ++++++++-------------------
  1 file changed, 8 insertions(+), 19 deletions(-)
crypto/gf128mul.c +8 -19
index 5276607..90cf17d 100644
@@ -88,29 +88,18 @@
  	q(0xf8), q(0xf9), q(0xfa), q(0xfb), q(0xfc), q(0xfd), q(0xfe), q(0xff) \
  }
 
-/*	Given the value i in 0..255 as the byte overflow when a field element
-    in GHASH is multiplied by x^8, this function will return the values that
-    are generated in the lo 16-bit word of the field value by applying the
-    modular polynomial. The values lo_byte and hi_byte are returned via the
-    macro xp_fun(lo_byte, hi_byte) so that the values can be assembled into
-    memory as required by a suitable definition of this macro operating on
-    the table above
-*/
-
-#define xx(p, q)	0x##p##q
-
  #define xda_bbe(i) ( \
-	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \
-	(i & 0x20 ? xx(10, e0) : 0) ^ (i & 0x10 ? xx(08, 70) : 0) ^ \
-	(i & 0x08 ? xx(04, 38) : 0) ^ (i & 0x04 ? xx(02, 1c) : 0) ^ \
-	(i & 0x02 ? xx(01, 0e) : 0) ^ (i & 0x01 ? xx(00, 87) : 0) \
+	(i & 0x80 ? 0x4380 : 0) ^ (i & 0x40 ? 0x21c0 : 0) ^ \
+	(i & 0x20 ? 0x10e0 : 0) ^ (i & 0x10 ? 0x0870 : 0) ^ \
+	(i & 0x08 ? 0x0438 : 0) ^ (i & 0x04 ? 0x021c : 0) ^ \
+	(i & 0x02 ? 0x010e : 0) ^ (i & 0x01 ? 0x0087 : 0) \
  )
 
  #define xda_lle(i) ( \
-	(i & 0x80 ? xx(e1, 00) : 0) ^ (i & 0x40 ? xx(70, 80) : 0) ^ \
-	(i & 0x20 ? xx(38, 40) : 0) ^ (i & 0x10 ? xx(1c, 20) : 0) ^ \
-	(i & 0x08 ? xx(0e, 10) : 0) ^ (i & 0x04 ? xx(07, 08) : 0) ^ \
-	(i & 0x02 ? xx(03, 84) : 0) ^ (i & 0x01 ? xx(01, c2) : 0) \
+	(i & 0x80 ? 0xe100 : 0) ^ (i & 0x40 ? 0x7080 : 0) ^ \
+	(i & 0x20 ? 0x3840 : 0) ^ (i & 0x10 ? 0x1c20 : 0) ^ \
+	(i & 0x08 ? 0x0e10 : 0) ^ (i & 0x04 ? 0x0708 : 0) ^ \
+	(i & 0x02 ? 0x0384 : 0) ^ (i & 0x01 ? 0x01c2 : 0) \
  )
 
Quoted 5 lines

crypto/gf128mul.c line 105

Quoted 4 lines
crypto/gf128mul.c +0 -0
  static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);

Hi there,

Hi Mike.

Quoted 2 lines

I'm not really contributing anything other than getting code checkpatch clean, whilst I relearn C and get a feel for various parts of the kernel.

checkpatch cleanliness, while OK for some parts of the kernel, should not be an end-goal.

checkpatch is really a tool to "check patches".

If you want to submit neatening only patches, please do your changes in drivers/staging/

Otherwise, please look for code that isn't simply a style neatening bit, but something that actively makes reading the code easier, makes the object code smaller or faster, reduces complexity, adds extensibility, etc, etc...

cheers, Joe

#4 lore raw
Quoted 86 lines · Joe Perches
Quoted 68 lines · Michael Roocroft
Quoted 67 lines · Joe Perches
Quoted 1 line · Mike Roocroft

Fixed a coding style issue.

[]

crypto/gf128mul.c

Quoted 1 line
crypto/gf128mul.c +0 -0

[]

crypto/gf128mul.c line 103

Quoted 9 lines
crypto/gf128mul.c +1 -1
@@ -97,7 +97,7 @@
       the table above
   */
 
-#define xx(p, q)	0x##p##q
+#define xx(p, q)	(0x##p##q)
 
   #define xda_bbe(i) ( \
   	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \

I think that macro is pretty useless and nothing
but obfuscation now.

The comment above it doesn't seem useful either.

How about just removing and replacing the uses
like this?

---
   crypto/gf128mul.c | 27 ++++++++-------------------
   1 file changed, 8 insertions(+), 19 deletions(-)
crypto/gf128mul.c +8 -19
index 5276607..90cf17d 100644
@@ -88,29 +88,18 @@
   	q(0xf8), q(0xf9), q(0xfa), q(0xfb), q(0xfc), q(0xfd), q(0xfe), q(0xff) \
   }
 
-/*	Given the value i in 0..255 as the byte overflow when a field element
-    in GHASH is multiplied by x^8, this function will return the values that
-    are generated in the lo 16-bit word of the field value by applying the
-    modular polynomial. The values lo_byte and hi_byte are returned via the
-    macro xp_fun(lo_byte, hi_byte) so that the values can be assembled into
-    memory as required by a suitable definition of this macro operating on
-    the table above
-*/
-
-#define xx(p, q)	0x##p##q
-
   #define xda_bbe(i) ( \
-	(i & 0x80 ? xx(43, 80) : 0) ^ (i & 0x40 ? xx(21, c0) : 0) ^ \
-	(i & 0x20 ? xx(10, e0) : 0) ^ (i & 0x10 ? xx(08, 70) : 0) ^ \
-	(i & 0x08 ? xx(04, 38) : 0) ^ (i & 0x04 ? xx(02, 1c) : 0) ^ \
-	(i & 0x02 ? xx(01, 0e) : 0) ^ (i & 0x01 ? xx(00, 87) : 0) \
+	(i & 0x80 ? 0x4380 : 0) ^ (i & 0x40 ? 0x21c0 : 0) ^ \
+	(i & 0x20 ? 0x10e0 : 0) ^ (i & 0x10 ? 0x0870 : 0) ^ \
+	(i & 0x08 ? 0x0438 : 0) ^ (i & 0x04 ? 0x021c : 0) ^ \
+	(i & 0x02 ? 0x010e : 0) ^ (i & 0x01 ? 0x0087 : 0) \
   )
 
   #define xda_lle(i) ( \
-	(i & 0x80 ? xx(e1, 00) : 0) ^ (i & 0x40 ? xx(70, 80) : 0) ^ \
-	(i & 0x20 ? xx(38, 40) : 0) ^ (i & 0x10 ? xx(1c, 20) : 0) ^ \
-	(i & 0x08 ? xx(0e, 10) : 0) ^ (i & 0x04 ? xx(07, 08) : 0) ^ \
-	(i & 0x02 ? xx(03, 84) : 0) ^ (i & 0x01 ? xx(01, c2) : 0) \
+	(i & 0x80 ? 0xe100 : 0) ^ (i & 0x40 ? 0x7080 : 0) ^ \
+	(i & 0x20 ? 0x3840 : 0) ^ (i & 0x10 ? 0x1c20 : 0) ^ \
+	(i & 0x08 ? 0x0e10 : 0) ^ (i & 0x04 ? 0x0708 : 0) ^ \
+	(i & 0x02 ? 0x0384 : 0) ^ (i & 0x01 ? 0x01c2 : 0) \
   )
 
Quoted 5 lines

crypto/gf128mul.c line 105

Quoted 4 lines
crypto/gf128mul.c +0 -0
   static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);

Hi there,

Hi Mike.

Quoted 2 lines

I'm not really contributing anything other than getting code checkpatch clean, whilst I relearn C and get a feel for various parts of the kernel.

checkpatch cleanliness, while OK for some parts of the kernel, should not be an end-goal.

checkpatch is really a tool to "check patches".

If you want to submit neatening only patches, please do your changes in drivers/staging/

Quoted 9 lines

Otherwise, please look for code that isn't simply a style neatening bit, but something that actively makes reading the code easier, makes the object code smaller or faster, reduces complexity, adds extensibility, etc, etc...

cheers, Joe

Hi Joe

Thanks for the Advice, I fully intend to making more meaningful contributions when my confidence in writing C is better than it is at the moment. I'll concentrate on making any changes to staging whilst I learn and get to grips with git, and continue to look through the rest of the kernel tree as a learning exercise.

I am extremely new to all this and a little overwhelmed, but by looking and not doing anyhing i'll never learn anything.

#5 lore raw
Quoted 4 lines · Michael Roocroft

I fully intend to making more meaningful contributions
when my confidence in writing C is better than it is at the moment. I'll concentrate on making any changes to staging whilst I learn and get to grips with git, and continue to look through the rest of the kernel tree as a learning exercise.

Sounds like a good plan to me. welcome btw.

Quoted 1 line

by looking and not doing anything i'll never learn anything.

True words...

cheers, Joe