Skip to contentSkip to conversation

Creates macro to avoid variable shadowing

PATCH In review Leonardo Brás 3 participants 6 messages Oct 23, 2018 – Oct 30, 2018 Kbuild

lore
Share & tools
lore

Leonardo Brás

author
#0 lore raw

PATCH v2 3/5 Creates macro to avoid variable shadowing

Creates DEF_FIELD_ADDR_VAR as a more generic version of the DEF_FIELD_ADD macro, allowing usage of a variable name other than the struct element name. Also, sets DEF_FIELD_ADDR as a specific usage of DEF_FILD_ADDR_VAR in which the var name is the same as the struct element name.

Signed-off-by Leonardo Brás <[email protected]>
---
 scripts/mod/file2alias.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
scripts/mod/file2alias.c +17 -7
index 7be43697ff84..3015c0bdecb2 100644
@@ -95,12 +95,20 @@ extern struct devtable *__start___devtable[], *__stop___devtable[];
  */
 #define DEF_FIELD(m, devid, f) \
 	typeof(((struct devid *)0)->f) f = TO_NATIVE(*(typeof(f) *)((m) + OFF_##devid##_##f))
+
+/* Define a variable v that holds the address of field f of struct devid
+ * based at address m.  Due to the way typeof works, for a field of type
+ * T[N] the variable has type T(*)[N], _not_ T*.
+ */
+#define DEF_FIELD_ADDR_VAR(m, devid, f, v) \
+	typeof(((struct devid *)0)->f) *v = ((m) + OFF_##devid##_##f)
+
 /* Define a variable f that holds the address of field f of struct devid
  * based at address m.  Due to the way typeof works, for a field of type
  * T[N] the variable has type T(*)[N], _not_ T*.
  */
 #define DEF_FIELD_ADDR(m, devid, f) \
-	typeof(((struct devid *)0)->f) *f = ((m) + OFF_##devid##_##f)
+	DEF_FIELD_ADDR_VAR(m, devid, f, f)
 
 /* Add a table entry.  We test function type matches while we're here. */
 #define ADD_TO_DEVTABLE(device_id, type, function) \

@@ -641,25 +649,27 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
 	unsigned int i;
 
 	device_id_check(mod->name, "pnp", size, id_size, symval);
+	DEF_FIELD_ADDR(symval, pnp_card_device_id, devs);
+	typeof(devs) devs_last;
 
 	for (i = 0; i < count; i++) {
 		unsigned int j;
-		DEF_FIELD_ADDR(symval + i*id_size, pnp_card_device_id, devs);
+		devs_last = devs + i * id_size;
 
 		for (j = 0; j < PNP_MAX_DEVICES; j++) {
-			const char *id = (char *)(*devs)[j].id;
-			int i2, j2;
+			const char *id = (char *)(*devs_last)[j].id;
+			int j2;
 			int dup = 0;
 
 			if (!id[0])
 				break;
 
 			/* find duplicate, already added value */
-			for (i2 = 0; i2 < i && !dup; i2++) {
-				DEF_FIELD_ADDR(symval + i2*id_size, pnp_card_device_id, devs);
+			while ((devs_last -= id_size) >= devs && !dup) {
 
 				for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
-					const char *id2 = (char *)(*devs)[j2].id;
+					const char *id2 =
+						(char *)(*devs_last)[j2].id;
 
 					if (!id2[0])
 						break;
Signature · 1 line
2.19.1
#1 lore raw

Hi Leonardo,

Thanks for your patch.

Quoted 15 lines · Leonardo Brás

Creates DEF_FIELD_ADDR_VAR as a more generic version of the DEF_FIELD_ADD macro, allowing usage of a variable name other than the struct element name. Also, sets DEF_FIELD_ADDR as a specific usage of DEF_FILD_ADDR_VAR in which the var name is the same as the struct element name.

Signed-off-by Leonardo Brás <[email protected]>
---
 scripts/mod/file2alias.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
scripts/mod/file2alias.c +0 -0
index 7be43697ff84..3015c0bdecb2 100644

scripts/mod/file2alias.c extern struct devtable *__start___devtable[], *__stop___devtable[]; · line 104

Quoted 10 lines
scripts/mod/file2alias.c +7 -0
@@ -95,12 +95,20 @@ extern struct devtable *__start___devtable[], *__stop___devtable[];
  */
 #define DEF_FIELD(m, devid, f) \
 	typeof(((struct devid *)0)->f) f = TO_NATIVE(*(typeof(f) *)((m) + OFF_##devid##_##f))
+
+/* Define a variable v that holds the address of field f of struct devid
+ * based at address m.  Due to the way typeof works, for a field of type
+ * T[N] the variable has type T(*)[N], _not_ T*.
+ */
+#define DEF_FIELD_ADDR_VAR(m, devid, f, v) \
+	typeof(((struct devid *)0)->f) *v = ((m) + OFF_##devid##_##f)

You created this macro but you don't use it below, maybe you forgot to update the patch? Please check.

Helen

Quoted 46 lines
scripts/mod/file2alias.c +10 -7
+
 /* Define a variable f that holds the address of field f of struct devid
  * based at address m.  Due to the way typeof works, for a field of type
  * T[N] the variable has type T(*)[N], _not_ T*.
  */
 #define DEF_FIELD_ADDR(m, devid, f) \
-	typeof(((struct devid *)0)->f) *f = ((m) + OFF_##devid##_##f)
+	DEF_FIELD_ADDR_VAR(m, devid, f, f)
 
 /* Add a table entry.  We test function type matches while we're here. */
 #define ADD_TO_DEVTABLE(device_id, type, function) \

@@ -641,25 +649,27 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
 	unsigned int i;
 
 	device_id_check(mod->name, "pnp", size, id_size, symval);
+	DEF_FIELD_ADDR(symval, pnp_card_device_id, devs);
+	typeof(devs) devs_last;
 
 	for (i = 0; i < count; i++) {
 		unsigned int j;
-		DEF_FIELD_ADDR(symval + i*id_size, pnp_card_device_id, devs);
+		devs_last = devs + i * id_size;
 
 		for (j = 0; j < PNP_MAX_DEVICES; j++) {
-			const char *id = (char *)(*devs)[j].id;
-			int i2, j2;
+			const char *id = (char *)(*devs_last)[j].id;
+			int j2;
 			int dup = 0;
 
 			if (!id[0])
 				break;
 
 			/* find duplicate, already added value */
-			for (i2 = 0; i2 < i && !dup; i2++) {
-				DEF_FIELD_ADDR(symval + i2*id_size, pnp_card_device_id, devs);
+			while ((devs_last -= id_size) >= devs && !dup) {
 
 				for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
-					const char *id2 = (char *)(*devs)[j2].id;
+					const char *id2 =
+						(char *)(*devs_last)[j2].id;
 
 					if (!id2[0])
 						break;
#2 lore raw
In reply to Leonardo Brás

Hi Leonardo,

Quoted 37 lines · Leonardo Brás

Creates DEF_FIELD_ADDR_VAR as a more generic version of the DEF_FIELD_ADD macro, allowing usage of a variable name other than the struct element name. Also, sets DEF_FIELD_ADDR as a specific usage of DEF_FILD_ADDR_VAR in which the var name is the same as the struct element name.

Signed-off-by Leonardo Brás <[email protected]>
---
 scripts/mod/file2alias.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
scripts/mod/file2alias.c +9 -1
index 7be43697ff84..3015c0bdecb2 100644
@@ -95,12 +95,20 @@ extern struct devtable *__start___devtable[], *__stop___devtable[];
  */
 #define DEF_FIELD(m, devid, f) \
 	typeof(((struct devid *)0)->f) f = TO_NATIVE(*(typeof(f) *)((m) + OFF_##devid##_##f))
+
+/* Define a variable v that holds the address of field f of struct devid
+ * based at address m.  Due to the way typeof works, for a field of type
+ * T[N] the variable has type T(*)[N], _not_ T*.
+ */
+#define DEF_FIELD_ADDR_VAR(m, devid, f, v) \
+	typeof(((struct devid *)0)->f) *v = ((m) + OFF_##devid##_##f)
+
 /* Define a variable f that holds the address of field f of struct devid
  * based at address m.  Due to the way typeof works, for a field of type
  * T[N] the variable has type T(*)[N], _not_ T*.
  */
 #define DEF_FIELD_ADDR(m, devid, f) \
-	typeof(((struct devid *)0)->f) *f = ((m) + OFF_##devid##_##f)
+	DEF_FIELD_ADDR_VAR(m, devid, f, f)
 
 /* Add a table entry.  We test function type matches while we're here. */
 #define ADD_TO_DEVTABLE(device_id, type, function) \

scripts/mod/file2alias.c static void do_pnp_card_entries(void *symval, unsigned long size, · line 657

Quoted 10 lines
scripts/mod/file2alias.c +3 -1
@@ -641,25 +649,27 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
 	unsigned int i;
 
 	device_id_check(mod->name, "pnp", size, id_size, symval);
+	DEF_FIELD_ADDR(symval, pnp_card_device_id, devs);
+	typeof(devs) devs_last;
 
 	for (i = 0; i < count; i++) {
 		unsigned int j;
-		DEF_FIELD_ADDR(symval + i*id_size, pnp_card_device_id, devs);
+		devs_last = devs + i * id_size;

Instead of doing all these changes, you could just modify here with
DEF_FIELD_ADDR_VAR(symval + i*id_size, pnp_card_device_id, devs,
devs_aux);

Like this you are not changing the logic in this file, you are just renaming the variable which is more then enough to solve the -Wshadow warning.
I don't it is worthy changing the logic.

Regards,
Helen.

Quoted 24 lines
scripts/mod/file2alias.c +5 -5
 		for (j = 0; j < PNP_MAX_DEVICES; j++) {
-			const char *id = (char *)(*devs)[j].id;
-			int i2, j2;
+			const char *id = (char *)(*devs_last)[j].id;
+			int j2;
 			int dup = 0;
 
 			if (!id[0])
 				break;
 
 			/* find duplicate, already added value */
-			for (i2 = 0; i2 < i && !dup; i2++) {
-				DEF_FIELD_ADDR(symval + i2*id_size, pnp_card_device_id, devs);
+			while ((devs_last -= id_size) >= devs && !dup) {
 
 				for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
-					const char *id2 = (char *)(*devs)[j2].id;
+					const char *id2 =
+						(char *)(*devs_last)[j2].id;
 
 					if (!id2[0])
 						break;
author
#3 lore raw

Hello Helen,

Quoted 4 lines · Helen Koike

Like this you are not changing the logic in this file, you are just renaming the variable which is more then enough to solve the -Wshadow warning.
I don't it is worthy changing the logic.

You are right, that was what the patch was supposed to do. For some crazy reason I must have submitted the wrong commit as a patch.

I will fix it and send you all a v3 ASAP.

Sorry for the inconvenience.
Thanks!

Leonardo Brás

#4 lore raw
In reply to Leonardo Brás
Quoted 8 lines · Leonardo Brás

Creates DEF_FIELD_ADDR_VAR as a more generic version of the DEF_FIELD_ADD macro, allowing usage of a variable name other than the struct element name. Also, sets DEF_FIELD_ADDR as a specific usage of DEF_FILD_ADDR_VAR in which the var name is the same as the struct element name.

Signed-off-by Leonardo Brás <[email protected]>

---

Applied to linux-kbuild.

Quoted 66 lines
 scripts/mod/file2alias.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
scripts/mod/file2alias.c +17 -8
index 7be43697ff84..3015c0bdecb2 100644
@@ -95,12 +95,20 @@ extern struct devtable *__start___devtable[], *__stop___devtable[];
  */
 #define DEF_FIELD(m, devid, f) \
        typeof(((struct devid *)0)->f) f = TO_NATIVE(*(typeof(f) *)((m) + OFF_##devid##_##f))
+
+/* Define a variable v that holds the address of field f of struct devid
+ * based at address m.  Due to the way typeof works, for a field of type
+ * T[N] the variable has type T(*)[N], _not_ T*.
+ */
+#define DEF_FIELD_ADDR_VAR(m, devid, f, v) \
+       typeof(((struct devid *)0)->f) *v = ((m) + OFF_##devid##_##f)
+
 /* Define a variable f that holds the address of field f of struct devid
  * based at address m.  Due to the way typeof works, for a field of type
  * T[N] the variable has type T(*)[N], _not_ T*.
  */
 #define DEF_FIELD_ADDR(m, devid, f) \
-       typeof(((struct devid *)0)->f) *f = ((m) + OFF_##devid##_##f)
+       DEF_FIELD_ADDR_VAR(m, devid, f, f)
 
 /* Add a table entry.  We test function type matches while we're here. */
 #define ADD_TO_DEVTABLE(device_id, type, function) \

@@ -641,25 +649,27 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
        unsigned int i;
 
        device_id_check(mod->name, "pnp", size, id_size, symval);
+       DEF_FIELD_ADDR(symval, pnp_card_device_id, devs);
+       typeof(devs) devs_last;
 
        for (i = 0; i < count; i++) {
                unsigned int j;
-               DEF_FIELD_ADDR(symval + i*id_size, pnp_card_device_id, devs);
+               devs_last = devs + i * id_size;
 
                for (j = 0; j < PNP_MAX_DEVICES; j++) {
-                       const char *id = (char *)(*devs)[j].id;
-                       int i2, j2;
+                       const char *id = (char *)(*devs_last)[j].id;
+                       int j2;
                        int dup = 0;
 
                        if (!id[0])
                                break;
 
                        /* find duplicate, already added value */
-                       for (i2 = 0; i2 < i && !dup; i2++) {
-                               DEF_FIELD_ADDR(symval + i2*id_size, pnp_card_device_id, devs);
+                       while ((devs_last -= id_size) >= devs && !dup) {
 
                                for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
-                                       const char *id2 = (char *)(*devs)[j2].id;
+                                       const char *id2 =
+                                               (char *)(*devs_last)[j2].id;
 
                                        if (!id2[0])
                                                break;
--

2.19.1

Signature · 2 lines
Best Regards
Masahiro Yamada
author
#5 lore raw

Thank you!

Quoted 87 lines · Masahiro Yamada
Quoted 8 lines · Leonardo Brás

Creates DEF_FIELD_ADDR_VAR as a more generic version of the DEF_FIELD_ADD macro, allowing usage of a variable name other than the struct element name. Also, sets DEF_FIELD_ADDR as a specific usage of DEF_FILD_ADDR_VAR in which the var name is the same as the struct element name.

Signed-off-by Leonardo Brás <[email protected]>

---

Applied to linux-kbuild.

Quoted 64 lines
 scripts/mod/file2alias.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
scripts/mod/file2alias.c +17 -8
index 7be43697ff84..3015c0bdecb2 100644
@@ -95,12 +95,20 @@ extern struct devtable *__start___devtable[], *__stop___devtable[];
  */
 #define DEF_FIELD(m, devid, f) \
        typeof(((struct devid *)0)->f) f = TO_NATIVE(*(typeof(f) *)((m) + OFF_##devid##_##f))
+
+/* Define a variable v that holds the address of field f of struct devid
+ * based at address m.  Due to the way typeof works, for a field of type
+ * T[N] the variable has type T(*)[N], _not_ T*.
+ */
+#define DEF_FIELD_ADDR_VAR(m, devid, f, v) \
+       typeof(((struct devid *)0)->f) *v = ((m) + OFF_##devid##_##f)
+
 /* Define a variable f that holds the address of field f of struct devid
  * based at address m.  Due to the way typeof works, for a field of type
  * T[N] the variable has type T(*)[N], _not_ T*.
  */
 #define DEF_FIELD_ADDR(m, devid, f) \
-       typeof(((struct devid *)0)->f) *f = ((m) + OFF_##devid##_##f)
+       DEF_FIELD_ADDR_VAR(m, devid, f, f)
 
 /* Add a table entry.  We test function type matches while we're here. */
 #define ADD_TO_DEVTABLE(device_id, type, function) \

@@ -641,25 +649,27 @@ static void do_pnp_card_entries(void *symval, unsigned long size,
        unsigned int i;
 
        device_id_check(mod->name, "pnp", size, id_size, symval);
+       DEF_FIELD_ADDR(symval, pnp_card_device_id, devs);
+       typeof(devs) devs_last;
 
        for (i = 0; i < count; i++) {
                unsigned int j;
-               DEF_FIELD_ADDR(symval + i*id_size, pnp_card_device_id, devs);
+               devs_last = devs + i * id_size;
 
                for (j = 0; j < PNP_MAX_DEVICES; j++) {
-                       const char *id = (char *)(*devs)[j].id;
-                       int i2, j2;
+                       const char *id = (char *)(*devs_last)[j].id;
+                       int j2;
                        int dup = 0;
 
                        if (!id[0])
                                break;
 
                        /* find duplicate, already added value */
-                       for (i2 = 0; i2 < i && !dup; i2++) {
-                               DEF_FIELD_ADDR(symval + i2*id_size, pnp_card_device_id, devs);
+                       while ((devs_last -= id_size) >= devs && !dup) {
 
                                for (j2 = 0; j2 < PNP_MAX_DEVICES; j2++) {
-                                       const char *id2 = (char *)(*devs)[j2].id;
+                                       const char *id2 =
+                                               (char *)(*devs_last)[j2].id;
 
                                        if (!id2[0])
                                                break;
--
Quoted 2 lines

2.19.1

--
Best Regards
Masahiro Yamada