[U-Boot] [PATCH] [v2] powerpc/85xx: fix compatible property for the L2 cache node

[Wolfgang Denk]

Dear Timur Tabi,

In message <4DBB2723.4050408 at freescale.com> you wrote:
>
> I disagree.  It's quite clear what I'm trying to do.  I'm trying to insert a

This is your opinion. I disagree.

> NULL character into a string.  Since device tree properties use a NULL to
> delimit multiple strings, it's clear that this is what the "0" is for.

Wrong data type. In C strings are _terminated_ by '\0' characters, so
using functions that are designed to deal with C strings are
obviously not the right tool to deal with data structures that have
_embedded_ NUL characters.

If you try, it quickly gets ugly like the code I rejected.

For example, who gives you any guarantee that sprintf() will continue
to append characters after it inserted the first NUL character?
A clever implementation could optimize this and return immediately
after seeing a NUL...

> Look at the original code:
> 
> 	len = sprintf(compat_buf,
> 		"fsl,%c%s-l2-cache-controller",
> 		tolower(cpu->name[0]), cpu->name + 1);
> 
> 	sprintf(&compat_buf[len + 1], "cache");
> 
> I think my patch is clearer than this.  In fact, because the original code was
> so obscure, there was a bug in it.  I could have done this:

Why exactly do you think you have to use sprintf() to append a
constant string like "cache"?

If you want to make clean what's intended, then use something like
this:

	len = sprintf(print_buf, "fsl,%c%s-l2-cache-controller",
		tolower(cpu->name[0]), cpu->name + 1);
	
	/* Include NUL characters */
	memcpy(compat_buf, print_buf, len + 1);
	memcpy(compat_buf + len + 1, "cache", sizeof("cache"));

If you want to optimize (I'm a fan of small memory footprint, but I'm
also a fan of readable code), use

	len = sprintf(compat_buf, "fsl,%c%s-l2-cache-controller",
		tolower(cpu->name[0]), cpu->name + 1);
	
	/* Include NUL characters */
	memcpy(compat_buf + len + 1, "cache", sizeof("cache"));

etc.

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
egrep patterns are full regular expressions; it uses a fast  determi-
nistic algorithm that sometimes needs exponential space.
- unix manuals