Re: [PATCH 1/2] Enable a way to provide the reason for "being here"

[Juergen Beisert <jbe@pengutronix.de>]

Uwe Kleine-König wrote:
> On Wed, Jun 20, 2012 at 09:09:53PM +0200, Juergen Beisert wrote:
> > Hi Uwe,
> >
> > Uwe Kleine-König wrote:
> > > On Wed, Jun 20, 2012 at 05:08:52PM +0200, Juergen Beisert wrote:
> > > > Marc Kleine-Budde wrote:
> > > > > [...]
> > > > >
> > > > > > +
> > > > > > +#include <common.h>
> > > > > > +#include <init.h>
> > > > > > +#include <environment.h>
> > > > > > +#include <globalvar.h>
> > > > > > +#include <reset_source.h>
> > > > > > +
> > > > > > +static const char name[] = "global.system.reset";
> > > > > > +static const char unknown_reset[] = "unknown";
> > > > > > +static const char power_on_reset[] = "POR";
> > > > > > +static const char manual_reset[] = "RST";
> > > > > > +static const char watchdog[] = "WDG";
> > > > > > +static const char wake[] = "WKE";
> > > > > > +static const char jtag[] = "JTAG";
> > > > >
> > > > > what about using an array
> > > > >
> > > > > static cost char *reset_reason_array[] = {
> > > > > 	[RESET_UKWN] = "unknown",
> > > > > 	...
> > > > > };
> > > >
> > > > The result is not the same. The strings are no longer "const". But it
> > > > should
> > >
> > > <kidding>Of course they are not "const" if you write "cost".</kidding>
> > > Using
> > >
> > > 	static const char * const reset_reason_array[] = {
> > >
> > > should do the trick.
> >
> > No, it doesn't. Only this would:
> >
> > static const bla[] = "this is a really constant string";
> > static const char * const reset_reason_array[] = {
> > 	[0] = bla,
> > [...]
> > };
>
> What makes you think it's not const. I think it is, look:
>
> 	ukleinek@perseus:~/tmp$ cat test.c
> 	static const char * const const_reset_reason_array[] = {
> 		"jtag",
> 		"por",
> 	};
>
> 	static const char *reset_reason_array[] = {
> 		"jtag",
> 		"por",
> 	};
>
> 	void somefunc(void)
> 	{
> 		char *reason = const_reset_reason_array[0];
> 	}
> 	ukleinek@perseus:~/tmp$ gcc -Wall -c test.c
> 	test.c: In function ‘somefunc’:
> 	test.c:13:17: warning: initialization discards ‘const’ qualifier from
> pointer target type [enabled by default] test.c:13:8: warning: unused
> variable ‘reason’ [-Wunused-variable] test.c: At top level:
> 	test.c:6:20: warning: ‘reset_reason_array’ defined but not used
> [-Wunused-variable]
>
> 	ukleinek@perseus:~/tmp$ objdump -D -j .rodata -j .data test.o
>
> 	test.o:     file format elf64-x86-64
>
>
> 	Disassembly of section .data:
>
> 	0000000000000000 <reset_reason_array>:
> 		...
>
> 	Disassembly of section .rodata:
>
> 	0000000000000000 <const_reset_reason_array-0x10>:
> 	   0:	6a 74                	pushq  $0x74
> 	   2:	61                   	(bad)
> 	   3:	67 00 70 6f          	add    %dh,0x6f(%eax)
> 	   7:	72 00                	jb     9 <const_reset_reason_array-0x7>
> 	   9:	00 00                	add    %al,(%rax)
> 	   b:	00 00                	add    %al,(%rax)
> 	   d:	00 00                	add    %al,(%rax)
> 		...
>
> 	0000000000000010 <const_reset_reason_array>:
> 		...
>
> so const_reset_reason_array lives in .rodata and assinging
> const_reset_reason_array[0] to a char * results in a warning.
>
> What am I missing?

const char friesel[] = "this is really a constant string";
const char *frasel = "this is not really a constant string";

The compiler will place the "this is really a constant string" into 
the .rodata, but "this is not really a constant string" will be still 
in .data (writeable). This difference is very important to know, if you want 
the compiler to place the strings into the flash of a microcontroller instead 
of the RAM (which is most of the time very small).

Refer also section "2.4 Choosing the Right Type" in Ulrich Drepper's "How To 
Write Shared Libraries".

But for Barebox this difference is not very important.

jbe

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Vertretung Sued/Muenchen, Germany             | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox