[U-Boot] [PATCH 2/4] Use LINK_OFF to access global data

[Mike Frysinger]

On Wednesday 30 December 2009 10:08:30 Joakim Tjernlund wrote:
> --- a/common/cmd_nvedit.c
> +++ b/common/cmd_nvedit.c
> @@ -512,6 +512,7 @@ char *getenv (char *name)
>  {
>  	int i, nxt;
> 
> +	name = LINK_OFF(name);
>  	WATCHDOG_RESET();
> 
>  	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
> @@ -534,6 +535,7 @@ int getenv_r (char *name, char *buf, unsigned len)
>  {
>  	int i, nxt;
> 
> +	name = LINK_OFF(name);
>  	for (i=0; env_get_char(i) != '\0'; i=nxt+1) {
>  		int val, n;

you have no guarantee that getenv() is called with a const string which is in 
the .rodata section.  there's code that generates the env name in a buffer on 
the stack and gives that to getenv().  does LINK_OFF() still work then ?

> --- a/common/console.c
> +++ b/common/console.c
> @@ -346,7 +346,7 @@ void putc(const char c)
>  	}
>  }
> 
> -void puts(const char *s)
> +static void printf_puts(const char *s)
>  {
>  #ifdef CONFIG_SILENT_CONSOLE
>  	if (gd->flags & GD_FLG_SILENT)
> @@ -367,12 +367,18 @@ void puts(const char *s)
>  	}
>  }
> 
> +void puts(const char *s)
> +{
> +	printf_puts(LINK_OFF(s));
> +}

and if CONFIG_LINK_OFF isnt defined, does gcc correctly inline this ?  if not, 
i think there needs to be #ifdef CONFIG_LINK_OFF handling here.

> --- a/include/linux/ctype.h
> +++ b/include/linux/ctype.h
> 
> -extern unsigned char _ctype[];
> +extern const unsigned char _ctype[];
>
> --- a/lib_generic/ctype.c
> +++ b/lib_generic/ctype.c
> 
> -unsigned char _ctype[] = {
> +const unsigned char _ctype[] = {
>
> --- a/tools/updater/ctype.c
> +++ b/tools/updater/ctype.c
> 
> -unsigned char _ctype[] = {
> +const unsigned char _ctype[] = {

seems like these const changes should be split and pushed separately

> --- a/lib_generic/crc32.c
> +++ b/lib_generic/crc32.c
> @@ -156,6 +156,11 @@
>  */ uint32_t ZEXPORT crc32 (uint32_t crc, const Bytef *buf, uInt len) {
> +#ifdef LINK_OFF
> +    const uint32_t *crc_tab = LINK_OFF(crc_table);
> +#else
> +    const uint32_t *crc_tab = crc_table;
> +#endif

the patch 1/4 you posted always defines LINK_OFF.  it's CONFIG_LINK_OFF which 
is dynamic.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20091231/cc24036e/attachment.pgp