common/spl: Mark arguments as unused

PATCH New Vikram Narayanan 4 participants 11 messages Oct 23, 2012 – Oct 29, 2012

lore

Share & tools

lore

Vikram Narayanan

author

2012-10-23 10:05 UTC #0 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

As dummy{1,2} are not used anywhere, mark it with __maybe_unused

Signed-off-by Vikram Narayanan <[email protected]>

Cc Stefan Roese <[email protected]>

---
 common/spl/spl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

common/spl/spl.c +1 -1

index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
 }
 #endif
 
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused ulong dummy2)
 {
 	u32 boot_device;
 	debug(">>spl:board_init_r()\n");

Signature · 1 line

1.7.4.1

Stefan Roese

2012-10-23 10:26 UTC #1 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 14 lines · Vikram Narayanan

As dummy{1,2} are not used anywhere, mark it with __maybe_unused
Signed-off-by Vikram Narayanan <[email protected]>
Cc Stefan Roese <[email protected]>
---
 common/spl/spl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +0 -0
index 0d829c0..62fd3bd 100644

common/spl/spl.c static void spl_ram_load_image(void) · line 151

Quoted 8 lines

common/spl/spl.c +1 -1

@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
 }
 #endif
 
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused ulong dummy2)
 {
 	u32 boot_device;
 	debug(">>spl:board_init_r()\n");

Perhaps even __always_unused instead of __maybe_unused as these variables are never used?

Thanks,
Stefan

Vikram Narayanan

author

2012-10-23 10:55 UTC #2 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 17 lines · Stefan Roese

Quoted 16 lines · Vikram Narayanan
As dummy{1,2} are not used anywhere, mark it with __maybe_unused
Signed-off-by Vikram Narayanan<[email protected]>
Cc Stefan Roese<[email protected]>
---
  common/spl/spl.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +0 -0
index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
  }
  #endif
 

Quoted 9 lines

common/spl/spl.c static void spl_ram_load_image(void) · line 151
Quoted 6 lines
common/spl/spl.c +1 -1
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused ulong dummy2)
  {
  	u32 boot_device;
  	debug(">>spl:board_init_r()\n");
Perhaps even __always_unused instead of __maybe_unused as these variables are never used?

Yes. It is better, in the case if it is never used at all.

Thanks,
Vikram

Tom Rini

2012-10-23 15:45 UTC #3 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

In reply to Stefan Roese

Quoted 17 lines · Stefan Roese

Quoted 16 lines · Vikram Narayanan
As dummy{1,2} are not used anywhere, mark it with __maybe_unused
Signed-off-by Vikram Narayanan <[email protected]>
Cc Stefan Roese <[email protected]>
---
 common/spl/spl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +0 -0
index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
 }
 #endif
 

Quoted 9 lines

common/spl/spl.c static void spl_ram_load_image(void) · line 151
Quoted 6 lines
common/spl/spl.c +1 -1
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused ulong dummy2)
 {
 	u32 boot_device;
 	debug(">>spl:board_init_r()\n");
Perhaps even __always_unused instead of __maybe_unused as these variables are never used?

Also, what does this give us? Fixing a sparse warning?

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20121023/9c8bf30b/attachment.pgp>

Vikram Narayanan

author

2012-10-23 17:15 UTC #4 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 28 lines · Tom Rini

Quoted 17 lines · Stefan Roese
Quoted 16 lines · Vikram Narayanan
As dummy{1,2} are not used anywhere, mark it with __maybe_unused
Signed-off-by Vikram Narayanan<[email protected]>
Cc Stefan Roese<[email protected]>
---
  common/spl/spl.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +0 -0
index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
  }
  #endif
 
Quoted 9 lines
common/spl/spl.c static void spl_ram_load_image(void) · line 151
Quoted 6 lines
common/spl/spl.c +1 -1
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused ulong dummy2)
  {
  	u32 boot_device;
  	debug(">>spl:board_init_r()\n");
Perhaps even __always_unused instead of __maybe_unused as these variables are never used?

Quoted 1 line

Also, what does this give us? Fixing a sparse warning?

Not a sparse warning. I noticed this while looking at the code.

~Vikram

Scott Wood

2012-10-24 01:52 UTC #5 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 30 lines · Vikram Narayanan

Quoted 29 lines · Tom Rini
Quoted 27 lines · Stefan Roese
Quoted 21 lines · Vikram Narayanan
As dummy{1,2} are not used anywhere, mark it with __maybe_unused
Signed-off-by Vikram Narayanan<[email protected]>
Cc Stefan Roese<[email protected]>
---
  common/spl/spl.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +1 -1
index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
  }
  #endif
 
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused  
 ulong dummy2)
  {
  	u32 boot_device;
Quoted 2 lines
  	debug(">>spl:board_init_r()\n");
Perhaps even __always_unused instead of __maybe_unused as these variables are never used?

Quoted 3 lines

Quoted 1 line
Also, what does this give us? Fixing a sparse warning?
Not a sparse warning. I noticed this while looking at the code.

If there's no warning, why do we need to ugly up the code with __maybe_unused?

Unused arguments are quite common, as a result of implementing a common interface where this implementation doesn't need all the information that the interface provides. It should not cause a warning and should not require annotation.

-Scott

Vikram Narayanan

author

2012-10-24 04:14 UTC #6 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 31 lines · Scott Wood

Quoted 30 lines · Vikram Narayanan
Quoted 29 lines · Tom Rini
Quoted 27 lines · Stefan Roese
Quoted 21 lines · Vikram Narayanan
As dummy{1,2} are not used anywhere, mark it with __maybe_unused
Signed-off-by Vikram Narayanan<[email protected]>
Cc Stefan Roese<[email protected]>
---
common/spl/spl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +1 -1
index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
 }
 #endif
 
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused
 ulong dummy2)
 {
 u32 boot_device;
Quoted 2 lines
debug(">>spl:board_init_r()\n");
Perhaps even __always_unused instead of __maybe_unused as these variables are never used?

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
Also, what does this give us? Fixing a sparse warning?
Not a sparse warning. I noticed this while looking at the code.
If there's no warning, why do we need to ugly up the code with __maybe_unused?

I'd rather call this a proper way of coding, than calling it ugly. But perceptions differ.

Quoted 4 lines

Unused arguments are quite common, as a result of implementing a common interface where this implementation doesn't need all the information that the interface provides. It should not cause a warning and should not require annotation.

~Vikram

Scott Wood

2012-10-24 17:11 UTC #7 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 34 lines · Vikram Narayanan

Quoted 33 lines · Scott Wood
Quoted 32 lines · Vikram Narayanan
Quoted 30 lines · Tom Rini
Quoted 17 lines · Stefan Roese
Quoted 16 lines · Vikram Narayanan
As dummy{1,2} are not used anywhere, mark it with __maybe_unused
Signed-off-by Vikram Narayanan<[email protected]>
Cc Stefan Roese<[email protected]>
---
common/spl/spl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +0 -0
index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
 }
 #endif
 
Quoted 10 lines
Quoted 7 lines
common/spl/spl.c +1 -1
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused
 ulong dummy2)
 {
 u32 boot_device;
debug(">>spl:board_init_r()\n");
Perhaps even __always_unused instead of __maybe_unused as these variables are never used?
Also, what does this give us? Fixing a sparse warning?

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
Not a sparse warning. I noticed this while looking at the code.
If there's no warning, why do we need to ugly up the code with __maybe_unused?
I'd rather call this a proper way of coding, than calling it ugly. But perceptions differ.

If you want to push for a change to the official coding style, and changing the warning options to go with it, go ahead (I'll argue against it of course), but until and unless you succeed at that, this isn't the way U-Boot code is written. I don't see a single instance of __maybe_unused in an argument list, or a single instance of __always_unused anywhere in U-Boot other than its definition. Unnecessary clutter is harmful to readability.

-Scott

Vikram Narayanan

author

2012-10-28 17:04 UTC #8 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 43 lines · Scott Wood

Quoted 34 lines · Vikram Narayanan
Quoted 33 lines · Scott Wood
Quoted 32 lines · Vikram Narayanan
Quoted 30 lines · Tom Rini
Quoted 17 lines · Stefan Roese
Quoted 16 lines · Vikram Narayanan
As dummy{1,2} are not used anywhere, mark it with __maybe_unused
Signed-off-by Vikram Narayanan<[email protected]>
Cc Stefan Roese<[email protected]>
---
common/spl/spl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +0 -0
index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
 }
 #endif
 
Quoted 10 lines
Quoted 7 lines
common/spl/spl.c +1 -1
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused
 ulong dummy2)
 {
 u32 boot_device;
debug(">>spl:board_init_r()\n");
Perhaps even __always_unused instead of __maybe_unused as these variables are never used?
Also, what does this give us? Fixing a sparse warning?
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
Not a sparse warning. I noticed this while looking at the code.
If there's no warning, why do we need to ugly up the code with __maybe_unused?
I'd rather call this a proper way of coding, than calling it ugly. But perceptions differ.

Quoted 7 lines

If you want to push for a change to the official coding style, and changing the warning options to go with it, go ahead (I'll argue against it of course), but until and unless you succeed at that, this isn't the way U-Boot code is written. I don't see a single instance of __maybe_unused in an argument list, or a single instance of __always_unused anywhere in U-Boot other than its definition. Unnecessary clutter is harmful to readability.

It's not worth arguing over a single line of code that isn't going to cause any significant change. That would save us both some time.

~Vikram

Scott Wood

2012-10-29 16:34 UTC #9 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 56 lines · Vikram Narayanan

Quoted 45 lines · Scott Wood
Quoted 35 lines · Vikram Narayanan
Quoted 34 lines · Scott Wood
Quoted 33 lines · Vikram Narayanan
Quoted 31 lines · Tom Rini
Quoted 18 lines · Stefan Roese
Quoted 17 lines · Vikram Narayanan
As dummy{1,2} are not used anywhere, mark it with
__maybe_unused
Signed-off-by Vikram Narayanan<[email protected]>
Cc Stefan Roese<[email protected]>
---
common/spl/spl.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
common/spl/spl.c +0 -0
index 0d829c0..62fd3bd 100644
@@ -145,7 +145,7 @@ static void spl_ram_load_image(void)
 }
 #endif
 
Quoted 10 lines
Quoted 7 lines
common/spl/spl.c +1 -1
-void board_init_r(gd_t *dummy1, ulong dummy2)
+void board_init_r(__maybe_unused gd_t *dummy1, __maybe_unused
 ulong dummy2)
 {
 u32 boot_device;
debug(">>spl:board_init_r()\n");
Perhaps even __always_unused instead of __maybe_unused as these variables are never used?
Also, what does this give us? Fixing a sparse warning?
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Not a sparse warning. I noticed this while looking at the code.
If there's no warning, why do we need to ugly up the code with __maybe_unused?
I'd rather call this a proper way of coding, than calling it ugly. But
perceptions differ.
Quoted 9 lines
If you want to push for a change to the official coding style, and changing the warning options to go with it, go ahead (I'll argue against
it of course), but until and unless you succeed at that, this isn't the
way U-Boot code is written. I don't see a single instance of __maybe_unused in an argument list, or a single instance of __always_unused anywhere in U-Boot other than its definition. Unnecessary clutter is harmful to readability.

Quoted 2 lines

It's not worth arguing over a single line of code that isn't going to cause any significant change. That would save us both some time.

So you're withdrawing the patch?

I'm more concerned about the precedent it sets than the one specific line.

-Scott

Vikram Narayanan

author

2012-10-29 18:54 UTC #10 lore raw

PATCH 1/1 common/spl: Mark arguments as unused

Quoted 5 lines · Scott Wood

Quoted 3 lines
It's not worth arguing over a single line of code that isn't going to cause any significant change. That would save us both some time.
So you're withdrawing the patch?

Yes.

Quoted 1 line

I'm more concerned about the precedent it sets than the one specific line.

Agree with your concern.

~Vikram