arm: Correct build error introduced by getenv_ulong() patch

Commit dc8bbea removed a local variable that is used in most ARM boards.

Since we want to avoid an 'unused variable' warning with later compilers, and the #ifdef logic of whether this variable is required is bit painful, this declares the variable local to the block of code that needs it.

Signed-off-by Simon Glass <[email protected]>

---
Changes in v2:
- Tidy up to call getenv() in declaration

 arch/arm/lib/board.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

Dear Simon Glass,

Quoted 11 lines · In message<[email protected]> you wrote:

Commit dc8bbea removed a local variable that is used in most ARM boards.

Since we want to avoid an 'unused variable' warning with later compilers, and the #ifdef logic of whether this variable is required is bit painful, this declares the variable local to the block of code that needs it.

Signed-off-by Simon Glass<[email protected]>

---
Changes in v2:
- Tidy up to call getenv() in declaration

Quoted 2 lines

  arch/arm/lib/board.c |   12 ++++++++----
  1 files changed, 8 insertions(+), 4 deletions(-)

Applied, thanks.

Best regards,

Wolfgang Denk

I've just fetched u-boot and I don't see this one. Can you push u-boot/master so that I can rebase u-boot-arm/master on it and launch some tests?

Dear Albert ARIBAUD,

Quoted 4 lines · In message<[email protected]> you wrote:

I've just fetched u-boot and I don't see this one. Can you push u-boot/master so that I can rebase u-boot-arm/master on it and launch some tests?

Done.

Best regards,

Wolfgang Denk

Le 25/10/2011 09:50, Wolfgang Denk a ?crit :

Quoted 9 lines

Dear Albert ARIBAUD,

Quoted 4 lines · In message<[email protected]> ?you wrote:

I've just fetched u-boot and I don't see this one. Can you push u-boot/master so that I can rebase u-boot-arm/master on it and launch some tests?

Quoted 1 line

Done.

Hmm... Weird. I've just fetched u-Boot again, and I cannot find this patch in there ; the last commit of u-boot/master seems to be 4962e38e9a4a053792722918bb11c5408549aebd, dated 2011-10-17.

Can somebody aside from me check this?

Quoted 4 lines

Best regards,

Wolfgang Denk

Amicalement,
--
Albert.

Quoted 5 lines

Quoted 4 lines

I've just fetched u-boot and I don't see this one. Can you push u-boot/master so that I can rebase u-boot-arm/master on it and launch some tests?

Quoted 1 line

Done.

Hmm... Weird. I've just fetched u-Boot again, and I cannot find this patch in there ; the last commit of u-boot/master seems to be 4962e38e9a4a053792722918bb11c5408549aebd, dated 2011-10-17.

Can somebody aside from me check this?

Dear Albert ARIBAUD,

Quoted 7 lines · In message <[email protected]> you wrote:

Quoted 6 lines

Quoted 4 lines

I've just fetched u-boot and I don't see this one. Can you push u-boot/master so that I can rebase u-boot-arm/master on it and launch some tests?

Done.

Quoted 5 lines

Hmm... Weird. I've just fetched u-Boot again, and I cannot find this patch in there ; the last commit of u-boot/master seems to be 4962e38e9a4a053792722918bb11c5408549aebd, dated 2011-10-17.

Can somebody aside from me check this?

Sorry, my fault. ?Should be fixed now.

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de If you're not part of the solution, you're part of the problem.

arch/arm/lib/board.c +2 -1

 
 	flash_size = flash_init();
 	if (flash_size > 0) {
 # ifdef CONFIG_SYS_FLASH_CHECKSUM
+		char *s = getenv("flashchecksum");
+
 		print_size(flash_size, "");
 		/*
 		 * Compute and print flash CRC if flashchecksum is set to 'y'
 		 *
 		 * NOTE: Maybe we should add some WATCHDOG_RESET()? XXX
 		 */
-		s = getenv("flashchecksum");
 		if (s && (*s == 'y')) {
 			printf("  CRC: %08X", crc32(0,
 				(const unsigned char *) CONFIG_SYS_FLASH_BASE,

@@ -566,9 +567,12 @@ void board_init_r(gd_t *id, ulong dest_addr)
 	/* Initialize from environment */
 	load_addr = getenv_ulong("loadaddr", 16, load_addr);
 #if defined(CONFIG_CMD_NET)

Quoted 24 lines · On Sunday 23 October 2011 23:44:35 Simon Glass wrote:

arch/arm/lib/board.c +0 -0

? ? ? flash_size = flash_init();
? ? ? if (flash_size > 0) {
?# ifdef CONFIG_SYS_FLASH_CHECKSUM
+ ? ? ? ? ? ? char *s = getenv("flashchecksum");
+
? ? ? ? ? ? ? print_size(flash_size, "");
? ? ? ? ? ? ? /*
? ? ? ? ? ? ? ?* Compute and print flash CRC if flashchecksum is set to 'y' ? ? ? ? ? ? ? ?*
? ? ? ? ? ? ? ?* NOTE: Maybe we should add some WATCHDOG_RESET()? XXX ? ? ? ? ? ? ? ?*/
- ? ? ? ? ? ? s = getenv("flashchecksum");
? ? ? ? ? ? ? if (s && (*s == 'y')) {
? ? ? ? ? ? ? ? ? ? ? printf(" ?CRC: %08X", crc32(0,
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (const unsigned char *) CONFIG_SYS_FLASH_BASE,

arch/arm/lib/board.c +0 -0

@@ -566,9 +567,12 @@ void board_init_r(gd_t *id, ulong dest_addr)

? ? ? /* Initialize from environment */
? ? ? load_addr = getenv_ulong("loadaddr", 16, load_addr); ?#if defined(CONFIG_CMD_NET)
- ? ? s = getenv("bootfile");
- ? ? if (s != NULL)

Quoted 8 lines

- ? ? ? ? ? ? copy_filename(BootFile, s, sizeof(BootFile));
+ ? ? {
+ ? ? ? ? ? ? char *s = getenv("bootfile");
+
+ ? ? ? ? ? ? if (s != NULL)
+ ? ? ? ? ? ? ? ? ? ? copy_filename(BootFile, s, sizeof(BootFile));
+ ? ? }
?#endif

seems like a better solution would be to use at the top:
? ? ? ?__maybe_unused char *s;

also, shouldn't these be "const char *s" ?
-mike

Quoted 37 lines · Mike Frysinger

Quoted 22 lines · On Sunday 23 October 2011 23:44:35 Simon Glass wrote:

arch/arm/lib/board.c +2 -0

 
      flash_size = flash_init();
      if (flash_size > 0) {
 # ifdef CONFIG_SYS_FLASH_CHECKSUM
+             char *s = getenv("flashchecksum");
+
              print_size(flash_size, "");
              /*
               * Compute and print flash CRC if flashchecksum is set to

'y' *
* NOTE: Maybe we should add some WATCHDOG_RESET()? XXX */

-             s = getenv("flashchecksum");
              if (s && (*s == 'y')) {
                      printf("  CRC: %08X", crc32(0,
                              (const unsigned char *)
CONFIG_SYS_FLASH_BASE, @@ -566,9 +567,12 @@ void board_init_r(gd_t *id,
ulong dest_addr) /* Initialize from environment */
      load_addr = getenv_ulong("loadaddr", 16, load_addr);

#if defined(CONFIG_CMD_NET)

arch/arm/lib/board.c void board_init_r(gd_t *id, ulong dest_addr) · line 576

Quoted 10 lines

arch/arm/lib/board.c +6 -3

-     s = getenv("bootfile");
-     if (s != NULL)
-             copy_filename(BootFile, s, sizeof(BootFile));
+     {
+             char *s = getenv("bootfile");
+
+             if (s != NULL)
+                     copy_filename(BootFile, s, sizeof(BootFile));
+     }
 #endif

seems like a better solution would be to use at the top:

       __maybe_unused char *s;

Quoted 1 line

also, shouldn't these be "const char *s" ?

We can certainly do this and I agree it is easier than #ifdefs. Does it introduce the possibility that one day the code will stop using the variable but it will still be declared? Is the fact that we need the #ifdefs an indication that the function should be too long and should be refactored? it in fact better to have these explicit so we can see them for the ugliness they are?

Quoted 38 lines · Mike Frysinger

Quoted 37 lines · On Sunday 23 October 2011 23:44:35 Simon Glass wrote:

Quoted 22 lines

arch/arm/lib/board.c +2 -0

 
      flash_size = flash_init();
      if (flash_size > 0) {
 # ifdef CONFIG_SYS_FLASH_CHECKSUM
+             char *s = getenv("flashchecksum");
+
              print_size(flash_size, "");
              /*
               * Compute and print flash CRC if flashchecksum is set to

'y' *
* NOTE: Maybe we should add some WATCHDOG_RESET()? XXX */

-             s = getenv("flashchecksum");
              if (s && (*s == 'y')) {
                      printf("  CRC: %08X", crc32(0,
                              (const unsigned char *)
CONFIG_SYS_FLASH_BASE, @@ -566,9 +567,12 @@ void board_init_r(gd_t *id,
ulong dest_addr) /* Initialize from environment */
      load_addr = getenv_ulong("loadaddr", 16, load_addr);

#if defined(CONFIG_CMD_NET)

arch/arm/lib/board.c void board_init_r(gd_t *id, ulong dest_addr) · line 576

Quoted 10 lines

arch/arm/lib/board.c +6 -3

-     s = getenv("bootfile");
-     if (s != NULL)
-             copy_filename(BootFile, s, sizeof(BootFile));
+     {
+             char *s = getenv("bootfile");
+
+             if (s != NULL)
+                     copy_filename(BootFile, s, sizeof(BootFile));
+     }
 #endif

seems like a better solution would be to use at the top:

       __maybe_unused char *s;

Quoted 8 lines

Quoted 1 line

also, shouldn't these be "const char *s" ?

We can certainly do this and I agree it is easier than #ifdefs. Does it introduce the possibility that one day the code will stop using the variable but it will still be declared? Is the fact that we need the #ifdefs an indication that the function should be too long and should be refactored? it in fact better to have these explicit so we can see them for the ugliness they are?

yes, you're right that it does leave the door open to the variable being declared, never used, and gcc not emitting a warning about it.

both setups suck, but i'd lean towards the less-ifdef state ... wonder if Wolfgang has a preference.

Hi Mike,

Quoted 48 lines · On Monday 31 October 2011 17:06:46 Simon Glass wrote:

Quoted 46 lines · Mike Frysinger

Quoted 37 lines · On Sunday 23 October 2011 23:44:35 Simon Glass wrote:

Quoted 24 lines

arch/arm/lib/board.c +0 -0

? ? ? flash_size = flash_init();
? ? ? if (flash_size > 0) {
?# ifdef CONFIG_SYS_FLASH_CHECKSUM
+ ? ? ? ? ? ? char *s = getenv("flashchecksum");
+
? ? ? ? ? ? ? print_size(flash_size, "");
? ? ? ? ? ? ? /*
? ? ? ? ? ? ? ?* Compute and print flash CRC if flashchecksum is set to 'y' *
? ? ? ? ? ? ? ?* NOTE: Maybe we should add some WATCHDOG_RESET()? XXX ? ? ? ? ? ? ? ?*/
- ? ? ? ? ? ? s = getenv("flashchecksum");
? ? ? ? ? ? ? if (s && (*s == 'y')) {
? ? ? ? ? ? ? ? ? ? ? printf(" ?CRC: %08X", crc32(0,
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (const unsigned char *) CONFIG_SYS_FLASH_BASE, @@ -566,9 +567,12 @@ void board_init_r(gd_t *id, ulong dest_addr) /* Initialize from environment */
? ? ? load_addr = getenv_ulong("loadaddr", 16, load_addr); ?#if defined(CONFIG_CMD_NET)
- ? ? s = getenv("bootfile");
- ? ? if (s != NULL)

Quoted 8 lines

- ? ? ? ? ? ? copy_filename(BootFile, s, sizeof(BootFile));
+ ? ? {
+ ? ? ? ? ? ? char *s = getenv("bootfile");
+
+ ? ? ? ? ? ? if (s != NULL)
+ ? ? ? ? ? ? ? ? ? ? copy_filename(BootFile, s, sizeof(BootFile));
+ ? ? }
?#endif

seems like a better solution would be to use at the top:
? ? ? ?__maybe_unused char *s;

Quoted 1 line

also, shouldn't these be "const char *s" ?

We can certainly do this and I agree it is easier than #ifdefs. Does it introduce the possibility that one day the code will stop using the variable but it will still be declared? Is the fact that we need the #ifdefs an indication that the function should be too long and should be refactored? it in fact better to have these explicit so we can see them for the ugliness they are?

Quoted 5 lines

yes, you're right that it does leave the door open to the variable being declared, never used, and gcc not emitting a warning about it.

both setups suck, but i'd lean towards the less-ifdef state ... wonder if Wolfgang has a preference.

Personally, I think that the nuisance of a potential unused variable is less of an issue than the actual _problems_ that ifdefs induce.

Cheers
?Detlev

--
The best way to predict the future is to invent it.
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? -- Alan Kay
--
DENX Software Engineering GmbH, ? ? ?MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, ?Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

Hi Detlev,

Quoted 56 lines · Detlev Zundel

Hi Mike,

Quoted 53 lines · On Monday 31 October 2011 17:06:46 Simon Glass wrote:

Quoted 38 lines · Mike Frysinger

Quoted 37 lines · On Sunday 23 October 2011 23:44:35 Simon Glass wrote:

Quoted 22 lines

arch/arm/lib/board.c +2 -0

 
       flash_size = flash_init();
       if (flash_size>  0) {
  # ifdef CONFIG_SYS_FLASH_CHECKSUM
+             char *s = getenv("flashchecksum");
+
               print_size(flash_size, "");
               /*
                * Compute and print flash CRC if flashchecksum is set to

'y' *
* NOTE: Maybe we should add some WATCHDOG_RESET()? XXX */

-             s = getenv("flashchecksum");
               if (s&&  (*s == 'y')) {
                       printf("  CRC: %08X", crc32(0,
                               (const unsigned char *)
CONFIG_SYS_FLASH_BASE, @@ -566,9 +567,12 @@ void board_init_r(gd_t *id,
ulong dest_addr) /* Initialize from environment */
       load_addr = getenv_ulong("loadaddr", 16, load_addr);

#if defined(CONFIG_CMD_NET)

arch/arm/lib/board.c void board_init_r(gd_t *id, ulong dest_addr) · line 576

Quoted 10 lines

arch/arm/lib/board.c +6 -3

-     s = getenv("bootfile");
-     if (s != NULL)
-             copy_filename(BootFile, s, sizeof(BootFile));
+     {
+             char *s = getenv("bootfile");
+
+             if (s != NULL)
+                     copy_filename(BootFile, s, sizeof(BootFile));
+     }
  #endif

seems like a better solution would be to use at the top:

        __maybe_unused char *s;

Quoted 8 lines

Quoted 1 line

also, shouldn't these be "const char *s" ?

We can certainly do this and I agree it is easier than #ifdefs. Does it introduce the possibility that one day the code will stop using the variable but it will still be declared? Is the fact that we need the #ifdefs an indication that the function should be too long and should be refactored? it in fact better to have these explicit so we can see them for the ugliness they are?

yes, you're right that it does leave the door open to the variable being declared, never used, and gcc not emitting a warning about it.

both setups suck, but i'd lean towards the less-ifdef state ... wonder if Wolfgang has a preference.

Quoted 2 lines

Personally, I think that the nuisance of a potential unused variable is less of an issue than the actual _problems_ that ifdefs induce.

Yes the #ifdefs are a pain. I am working on a replacement for board.c
- so far I have split things into functions. Next I need to look at Graeme's initcall patch.

Regards,
Simon

Le 08/11/2011 16:57, Simon Glass a ?crit :

Quoted 70 lines

Hi Detlev,

Quoted 65 lines · On Tue, Nov 8, 2011 at 1:20 AM, Detlev Zundel<[email protected]> ?wrote:

Hi Mike,

Quoted 53 lines · On Monday 31 October 2011 17:06:46 Simon Glass wrote:

Quoted 43 lines · Mike Frysinger

Quoted 41 lines · On Sunday 23 October 2011 23:44:35 Simon Glass wrote:

Quoted 27 lines

arch/arm/lib/board.c +0 -0

? ? ? flash_size = flash_init();
? ? ? if (flash_size> ?0) {
?# ifdef CONFIG_SYS_FLASH_CHECKSUM
+ ? ? ? ? ? ? char *s = getenv("flashchecksum");
+
? ? ? ? ? ? ? print_size(flash_size, "");
? ? ? ? ? ? ? /*
? ? ? ? ? ? ? ?* Compute and print flash CRC if flashchecksum is set to
'y' *
? ? ? ? ? ? ? ?* NOTE: Maybe we should add some WATCHDOG_RESET()? XXX ? ? ? ? ? ? ? ?*/
- ? ? ? ? ? ? s = getenv("flashchecksum");
? ? ? ? ? ? ? if (s&& ?(*s == 'y')) {
? ? ? ? ? ? ? ? ? ? ? printf(" ?CRC: %08X", crc32(0,
? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (const unsigned char *) CONFIG_SYS_FLASH_BASE, @@ -566,9 +567,12 @@ void board_init_r(gd_t *id,
ulong dest_addr) /* Initialize from environment */
? ? ? load_addr = getenv_ulong("loadaddr", 16, load_addr); ?#if defined(CONFIG_CMD_NET)
- ? ? s = getenv("bootfile");
- ? ? if (s != NULL)

Quoted 8 lines

- ? ? ? ? ? ? copy_filename(BootFile, s, sizeof(BootFile));
+ ? ? {
+ ? ? ? ? ? ? char *s = getenv("bootfile");
+
+ ? ? ? ? ? ? if (s != NULL)
+ ? ? ? ? ? ? ? ? ? ? copy_filename(BootFile, s, sizeof(BootFile));
+ ? ? }
?#endif

seems like a better solution would be to use at the top:
? ? ? ?__maybe_unused char *s;

Quoted 8 lines

Quoted 1 line

also, shouldn't these be "const char *s" ?

We can certainly do this and I agree it is easier than #ifdefs. Does it introduce the possibility that one day the code will stop using the variable but it will still be declared? Is the fact that we need the #ifdefs an indication that the function should be too long and should be refactored? it in fact better to have these explicit so we can see them for the ugliness they are?

Quoted 6 lines

yes, you're right that it does leave the door open to the variable being declared, never used, and gcc not emitting a warning about it.

both setups suck, but i'd lean towards the less-ifdef state ... wonder if
Wolfgang has a preference.

Personally, I think that the nuisance of a potential unused variable is less of an issue than the actual _problems_ that ifdefs induce.

Quoted 3 lines

Yes the #ifdefs are a pain. I am working on a replacement for board.c
- so far I have split things into functions. Next I need to look at Graeme's initcall patch.

I don't think 'ifdefs are' necessarily 'a pain'. They cater for a need, that is, to mark that some code depends on some condition. I find it *normal* that a checksum-related variable is conditioned on the checksum macro being defined.

Amicalement,
--
Albert.

This discussion was regarding the need to #ifdef the variable declaration, viz:

#if defined(THING1) || defined(THING2)
const char *cat;
#endif

...

#ifdef THING1
   cat = getenv("cat");

   send_back(cat);

#endif

....

#ifdef THING2
   cat = check_outside("cat");

   if (cat)
      wibble(cat);

#endif

and whether the top bit would be better as:

__maybe_unused const char *cat;

But more generally, lots of #ifdefs do make the code harder to read, and potentially more brittle in the face of config changes.

Dear Simon Glass,

Quoted 26 lines · In message <CAPnjgZ15f_gva5+MM1Em-L2sMxt1WAATxqiKUHOQAT893t9MMw@mail.gmail.com>…

This discussion was regarding the need to #ifdef the variable declaration, viz:

#if defined(THING1) || defined(THING2)
const char *cat;
#endif

...

#ifdef THING1
   cat = getenv("cat");

   send_back(cat);

#endif

....

#ifdef THING2
   cat = check_outside("cat");

   if (cat)
      wibble(cat);

#endif

Quoted 6 lines

and whether the top bit would be better as:

__maybe_unused const char *cat;

But more generally, lots of #ifdefs do make the code harder to read, and potentially more brittle in the face of config changes.

I would like to see only a minimal number of "__maybe_unused" in the code - in cases, where this is the way that hurts least.

In the examples above, it might be better to use local blocks, like:

#ifdef THING1
       {
               const char *cat = getenv("cat");

               send_back(cat);
       }

#endif

Hi Wolfgang

Quoted 41 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 26 lines · In message <CAPnjgZ15f_gva5+MM1Em-L2sMxt1WAATxqiKUHOQAT893t9MMw@mail.gmail.com>…

This discussion was regarding the need to #ifdef the variable declaration, viz:

#if defined(THING1) || defined(THING2)
const char *cat;
#endif

...

#ifdef THING1
   cat = getenv("cat");

   send_back(cat);

#endif

....

#ifdef THING2
   cat = check_outside("cat");

   if (cat)
      wibble(cat);

#endif

Quoted 6 lines

and whether the top bit would be better as:

__maybe_unused const char *cat;

But more generally, lots of #ifdefs do make the code harder to read, and potentially more brittle in the face of config changes.

I would like to see only a minimal number of "__maybe_unused" in the code - in cases, where this is the way that hurts least.

In the examples above, it might be better to use local blocks, like:

Quoted 7 lines

#ifdef THING1
       {
               const char *cat = getenv("cat");

               send_back(cat);
       }

#endif

I honestly think most of these cases can be factored out into functions. The compiler should inline them anyway so the overhead should be zero. The various board.c files are a prime example of where this should be done as a matter of principle to reduce the complexity and lenght of the primary function anyway

Hi Graeme,

Quoted 52 lines

Hi Wolfgang

Quoted 41 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 26 lines · In message <CAPnjgZ15f_gva5+MM1Em-L2sMxt1WAATxqiKUHOQAT893t9MMw@mail.gmail.com>…

This discussion was regarding the need to #ifdef the variable declaration, viz:

#if defined(THING1) || defined(THING2)
const char *cat;
#endif

...

#ifdef THING1
? ?cat = getenv("cat");

? ?send_back(cat);
#endif

....

#ifdef THING2
? ?cat = check_outside("cat");

? ?if (cat)
? ? ? wibble(cat);
#endif

Quoted 6 lines

and whether the top bit would be better as:

__maybe_unused const char *cat;

But more generally, lots of #ifdefs do make the code harder to read, and potentially more brittle in the face of config changes.

I ?would like to see only a minimal number of "__maybe_unused" in the code - in cases, where this is the way that hurts least.

In the examples above, it might be better to use local blocks, like:

Quoted 7 lines

#ifdef THING1
? ? ? ?{
? ? ? ? ? ? ? ?const char *cat = getenv("cat");

? ? ? ? ? ? ? ?send_back(cat);
? ? ? ?}
#endif

Quoted 5 lines

I honestly think most of these cases can be factored out into functions. The compiler should inline them anyway so the overhead should be zero. The various board.c files are a prime example of where this should be done as a matter of principle to reduce the complexity and lenght of the primary function anyway

I would even like to skip the ifdefs completely. ?Modern compilers with dead code elimination will completely do away unneeded code _but still do syntax checks on the parts every time_.

So maybe we should simply try to use

? ?if (THING1)
? ?{
? ? ? ? ...
? ?}

I know that this would need an "#ifdef THING1 1" but errors in this would be caught immediately (and not only under a certain combination of ifdefs) by the compiler so I don't think this is a problem.

I don't know how often I repeat my mantra, but every ifdef doubles the number of _different source codes_ that we deal with.

Cheers
?Detlev

--
Lotus Notes (GUI): ? Run away from it.
? ? ? ? ? ? ? ? ? ? ? ? ? -- linux/Documentation/email-clients.txt --
DENX Software Engineering GmbH, ? ? ?MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, ?Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Dear Simon Glass,

Quoted 26 lines · In message <CAPnjgZ15f_gva5+MM1Em-L2sMxt1WAATxqiKUHOQAT893t9MMw@mail.gmail.com>…

This discussion was regarding the need to #ifdef the variable declaration, viz:

#if defined(THING1) || defined(THING2)
const char *cat;
#endif

...

#ifdef THING1
? ?cat = getenv("cat");

? ?send_back(cat);
#endif

....

#ifdef THING2
? ?cat = check_outside("cat");

? ?if (cat)
? ? ? wibble(cat);
#endif

Quoted 6 lines

and whether the top bit would be better as:

__maybe_unused const char *cat;

But more generally, lots of #ifdefs do make the code harder to read, and potentially more brittle in the face of config changes.

I ?would like to see only a minimal number of "__maybe_unused" in the code - in cases, where this is the way that hurts least.

In the examples above, it might be better to use local blocks, like:

#ifdef THING1
? ? ? ?{
? ? ? ? ? ? ? ?const char *cat = getenv("cat");

? ? ? ? ? ? ? ?send_back(cat);
? ? ? ?}
#endif

Best regards,

Wolfgang Denk

--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de "There is such a fine line between genius and stupidity."
- David St. Hubbins, "Spinal Tap"