All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] Do not copy to same address
@ 2011-04-12  6:58 Matthias Weisser
  2011-04-12  7:05 ` Mike Frysinger
                   ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Matthias Weisser @ 2011-04-12  6:58 UTC (permalink / raw)
  To: u-boot

In some cases (e.g. bootm with a elf payload) there is a in place copy of
data to the same address. Catching this saves some ms while booting.

Signed-off-by: Matthias Weisser <weisserm@arcor.de>
---
 lib/string.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index b375b81..b8a9203 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -445,6 +445,9 @@ char * bcopy(const char * src, char * dest, int count)
 {
 	char *tmp = dest;
 
+	if (src == dest)
+		return dest;
+
 	while (count--)
 		*tmp++ = *src++;
 
@@ -467,6 +470,9 @@ void * memcpy(void *dest, const void *src, size_t count)
 	unsigned long *dl = (unsigned long *)dest, *sl = (unsigned long *)src;
 	char *d8, *s8;
 
+	if (src == dest)
+		return dest;
+
 	/* while all data is aligned (common case), copy a word at a time */
 	if ( (((ulong)dest | (ulong)src) & (sizeof(*dl) - 1)) == 0) {
 		while (count >= sizeof(*dl)) {
@@ -497,6 +503,9 @@ void * memmove(void * dest,const void *src,size_t count)
 {
 	char *tmp, *s;
 
+	if (src == dest)
+		return dest;
+
 	if (dest <= src) {
 		tmp = (char *) dest;
 		s = (char *) src;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH] Do not copy to same address
  2011-04-12  6:58 [U-Boot] [PATCH] Do not copy to same address Matthias Weisser
@ 2011-04-12  7:05 ` Mike Frysinger
  2011-04-12  7:16   ` Matthias Weißer
  2011-04-12  7:06 ` Albert ARIBAUD
  2011-05-23  9:03 ` [U-Boot] [PATCH V2] memcpy/memmove: " Matthias Weisser
  2 siblings, 1 reply; 22+ messages in thread
From: Mike Frysinger @ 2011-04-12  7:05 UTC (permalink / raw)
  To: u-boot

this summary is kind of weak.  please prefix it with something like "string:" 
or "memcpy/memmove:".  keep in mind that the summary needs to quickly pick out 
what the changeset is doing from every other changeset in the tree based only 
on that.  or at least give a pretty good idea.

side note, i wonder why we even bother with bcopy at all.  i dont see any 
users of it in the tree ...
-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/20110412/ea5eb6d6/attachment.pgp 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH] Do not copy to same address
  2011-04-12  6:58 [U-Boot] [PATCH] Do not copy to same address Matthias Weisser
  2011-04-12  7:05 ` Mike Frysinger
@ 2011-04-12  7:06 ` Albert ARIBAUD
  2011-04-12  7:13   ` Matthias Weißer
  2011-05-23  9:03 ` [U-Boot] [PATCH V2] memcpy/memmove: " Matthias Weisser
  2 siblings, 1 reply; 22+ messages in thread
From: Albert ARIBAUD @ 2011-04-12  7:06 UTC (permalink / raw)
  To: u-boot

Hi Matthias,

Le 12/04/2011 08:58, Matthias Weisser a ?crit :
> In some cases (e.g. bootm with a elf payload) there is a in place copy of
> data to the same address. Catching this saves some ms while booting.
>
> Signed-off-by: Matthias Weisser<weisserm@arcor.de>
> ---
>   lib/string.c |    9 +++++++++
>   1 files changed, 9 insertions(+), 0 deletions(-)

I believe that is a V2 patch, right? Please tag it as V2 in the subject 
line, and add patch history below the commit message delimiter ('---' ).

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH] Do not copy to same address
  2011-04-12  7:06 ` Albert ARIBAUD
@ 2011-04-12  7:13   ` Matthias Weißer
  2011-04-12  7:27     ` Albert ARIBAUD
  0 siblings, 1 reply; 22+ messages in thread
From: Matthias Weißer @ 2011-04-12  7:13 UTC (permalink / raw)
  To: u-boot

Am 12.04.2011 09:06, schrieb Albert ARIBAUD:
> Hi Matthias,
>
> Le 12/04/2011 08:58, Matthias Weisser a ?crit :
>> In some cases (e.g. bootm with a elf payload) there is a in place copy of
>> data to the same address. Catching this saves some ms while booting.
>>
>> Signed-off-by: Matthias Weisser<weisserm@arcor.de>
>> ---
>>    lib/string.c |    9 +++++++++
>>    1 files changed, 9 insertions(+), 0 deletions(-)
>
> I believe that is a V2 patch, right? Please tag it as V2 in the subject
> line, and add patch history below the commit message delimiter ('---' ).

No, it is a replacement for http://patchwork.ozlabs.org/patch/79447/ 
which picks up a suggestion from Wolfgang.

Regards,
Matthias

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH] Do not copy to same address
  2011-04-12  7:05 ` Mike Frysinger
@ 2011-04-12  7:16   ` Matthias Weißer
  0 siblings, 0 replies; 22+ messages in thread
From: Matthias Weißer @ 2011-04-12  7:16 UTC (permalink / raw)
  To: u-boot

Am 12.04.2011 09:05, schrieb Mike Frysinger:
> this summary is kind of weak.  please prefix it with something like "string:"
> or "memcpy/memmove:".  keep in mind that the summary needs to quickly pick out
> what the changeset is doing from every other changeset in the tree based only
> on that.  or at least give a pretty good idea.

OK. I will wait for additional comments and post a V2 then.

> side note, i wonder why we even bother with bcopy at all.  i dont see any
> users of it in the tree ...

I see the same. But removal of bcopy should be a separate patch.

Regards,
Matthias

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH] Do not copy to same address
  2011-04-12  7:13   ` Matthias Weißer
@ 2011-04-12  7:27     ` Albert ARIBAUD
  2011-04-12  7:49       ` Matthias Weißer
  0 siblings, 1 reply; 22+ messages in thread
From: Albert ARIBAUD @ 2011-04-12  7:27 UTC (permalink / raw)
  To: u-boot

Hi Matthias,

Le 12/04/2011 09:13, Matthias Wei?er a ?crit :
> Am 12.04.2011 09:06, schrieb Albert ARIBAUD:
>> Hi Matthias,
>>
>> Le 12/04/2011 08:58, Matthias Weisser a ?crit :
>>> In some cases (e.g. bootm with a elf payload) there is a in place
>>> copy of
>>> data to the same address. Catching this saves some ms while booting.
>>>
>>> Signed-off-by: Matthias Weisser<weisserm@arcor.de>
>>> ---
>>> lib/string.c | 9 +++++++++
>>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>
>> I believe that is a V2 patch, right? Please tag it as V2 in the subject
>> line, and add patch history below the commit message delimiter ('---' ).
>
> No, it is a replacement for http://patchwork.ozlabs.org/patch/79447/
> which picks up a suggestion from Wolfgang.

So it is a V3, not V2, but still you must tag it so that readers who see 
the previous patch can relate it to the 'replacement' -- yes, even if 
the files touched by V2 are different.

> Regards,
> Matthias

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH] Do not copy to same address
  2011-04-12  7:27     ` Albert ARIBAUD
@ 2011-04-12  7:49       ` Matthias Weißer
  2011-04-14  6:28         ` Albert ARIBAUD
  0 siblings, 1 reply; 22+ messages in thread
From: Matthias Weißer @ 2011-04-12  7:49 UTC (permalink / raw)
  To: u-boot

Am 12.04.2011 09:27, schrieb Albert ARIBAUD:
> Hi Matthias,
>
> Le 12/04/2011 09:13, Matthias Wei?er a ?crit :
>> Am 12.04.2011 09:06, schrieb Albert ARIBAUD:
>>> Hi Matthias,
>>>
>>> Le 12/04/2011 08:58, Matthias Weisser a ?crit :
>>>> In some cases (e.g. bootm with a elf payload) there is a in place
>>>> copy of
>>>> data to the same address. Catching this saves some ms while booting.
>>>>
>>>> Signed-off-by: Matthias Weisser<weisserm@arcor.de>
>>>> ---
>>>> lib/string.c | 9 +++++++++
>>>> 1 files changed, 9 insertions(+), 0 deletions(-)
>>>
>>> I believe that is a V2 patch, right? Please tag it as V2 in the subject
>>> line, and add patch history below the commit message delimiter ('---' ).
>>
>> No, it is a replacement for http://patchwork.ozlabs.org/patch/79447/
>> which picks up a suggestion from Wolfgang.
>
> So it is a V3, not V2, but still you must tag it so that readers who see
> the previous patch can relate it to the 'replacement' -- yes, even if
> the files touched by V2 are different.

Well, as the patch is only slightly related to my original one I thought 
it is better to start a new patch as I had to change the subject also. 
The only relation between them would be the reference in the mail 
header. Maybe Wolfgang can bring some light into this situation.

What would be the right way to post this patch? And what would be the 
right way to post a patch doing exactly the same to the ARM optimized 
version of memcpy?

Sorry for all that administrative questions.

Regards,
Matthias

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH] Do not copy to same address
  2011-04-12  7:49       ` Matthias Weißer
@ 2011-04-14  6:28         ` Albert ARIBAUD
  0 siblings, 0 replies; 22+ messages in thread
From: Albert ARIBAUD @ 2011-04-14  6:28 UTC (permalink / raw)
  To: u-boot

Hi Matthias,

Le 12/04/2011 09:49, Matthias Wei?er a ?crit :

> Well, as the patch is only slightly related to my original one I thought
> it is better to start a new patch as I had to change the subject also.
> The only relation between them would be the reference in the mail
> header. Maybe Wolfgang can bring some light into this situation.
>
> What would be the right way to post this patch?

Never mind with the previous patch, then; but please keep history of 
this one (maybe including a patchwork URL pointing to the previous patch 
with the other name) and tag its V2 in the subject line.

> And what would be the
> right way to post a patch doing exactly the same to the ARM optimized
> version of memcpy?

Simply submit a separate patch.

> Regards,
> Matthias

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-04-12  6:58 [U-Boot] [PATCH] Do not copy to same address Matthias Weisser
  2011-04-12  7:05 ` Mike Frysinger
  2011-04-12  7:06 ` Albert ARIBAUD
@ 2011-05-23  9:03 ` Matthias Weisser
  2011-05-23 21:07   ` Alexander Holler
                     ` (2 more replies)
  2 siblings, 3 replies; 22+ messages in thread
From: Matthias Weisser @ 2011-05-23  9:03 UTC (permalink / raw)
  To: u-boot

In some cases (e.g. bootm with a elf payload which is already at the right
position) there is a in place copy of data to the same address. Catching this
saves some ms while booting.

Signed-off-by: Matthias Weisser <weisserm@arcor.de>
---
Changes since V1:
  - Made subject more informative
  - Removed the optimization from bcopy as bcopy is not used anywhere
  
 lib/string.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index b375b81..2c4f0ec 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -467,6 +467,9 @@ void * memcpy(void *dest, const void *src, size_t count)
 	unsigned long *dl = (unsigned long *)dest, *sl = (unsigned long *)src;
 	char *d8, *s8;
 
+	if (src == dest)
+		return dest;
+
 	/* while all data is aligned (common case), copy a word at a time */
 	if ( (((ulong)dest | (ulong)src) & (sizeof(*dl) - 1)) == 0) {
 		while (count >= sizeof(*dl)) {
@@ -497,6 +500,9 @@ void * memmove(void * dest,const void *src,size_t count)
 {
 	char *tmp, *s;
 
+	if (src == dest)
+		return dest;
+
 	if (dest <= src) {
 		tmp = (char *) dest;
 		s = (char *) src;
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23  9:03 ` [U-Boot] [PATCH V2] memcpy/memmove: " Matthias Weisser
@ 2011-05-23 21:07   ` Alexander Holler
  2011-05-23 21:55     ` Wolfgang Denk
  2011-06-14  6:18   ` Matthias Weißer
  2011-07-25 22:28   ` Wolfgang Denk
  2 siblings, 1 reply; 22+ messages in thread
From: Alexander Holler @ 2011-05-23 21:07 UTC (permalink / raw)
  To: u-boot

Hello,

Am 23.05.2011 11:03, schrieb Matthias Weisser:
> In some cases (e.g. bootm with a elf payload which is already at the right
> position) there is a in place copy of data to the same address. Catching this
> saves some ms while booting.
>
> Signed-off-by: Matthias Weisser<weisserm@arcor.de>
> ---
> Changes since V1:
>    - Made subject more informative
>    - Removed the optimization from bcopy as bcopy is not used anywhere
>
>   lib/string.c |    6 ++++++
>   1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/lib/string.c b/lib/string.c
> index b375b81..2c4f0ec 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -467,6 +467,9 @@ void * memcpy(void *dest, const void *src, size_t count)
>   	unsigned long *dl = (unsigned long *)dest, *sl = (unsigned long *)src;
>   	char *d8, *s8;
>
> +	if (src == dest)
> +		return dest;
> +

here is the same, as in the patch I've commented before. There exist no 
reason to add a check for identity to memcpy() because memcpy doesn't 
support overlapping regions (and identity is just a special case of 
overlapping regions). If something might call memcpy() with overlapping 
or identical regions, it should use memmove().

>   	/* while all data is aligned (common case), copy a word at a time */
>   	if ( (((ulong)dest | (ulong)src)&  (sizeof(*dl) - 1)) == 0) {
>   		while (count>= sizeof(*dl)) {
> @@ -497,6 +500,9 @@ void * memmove(void * dest,const void *src,size_t count)
>   {
>   	char *tmp, *s;
>
> +	if (src == dest)
> +		return dest;
> +
>   	if (dest<= src) {

Here it is ok, but the check <= could be modified to < too.

Just to clarify my reasoning: the only reason why memcpy() exists, is 
because it should have been a faster version of memmove() without the 
necessary checks.
So if a bug proof of version of memcpy() is wanted, there is no need to 
have a different implementation for memcpy() and memcpy() could just be 
an alias for memmove().
But adding a check for identity to memcpy() is unnecessary.

Sorry, but I had to comment this after having read to many comments in a 
bug about something similiar in

https://bugzilla.redhat.com/show_bug.cgi?id=638477

(Be aware, reading that bug might hurt your brain)

Regards,

Alexander

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23 21:07   ` Alexander Holler
@ 2011-05-23 21:55     ` Wolfgang Denk
  2011-05-23 22:12       ` Alexander Holler
  0 siblings, 1 reply; 22+ messages in thread
From: Wolfgang Denk @ 2011-05-23 21:55 UTC (permalink / raw)
  To: u-boot

Dear Alexander Holler,

In message <4DDACC8B.6090507@ahsoftware.de> you wrote:
> 
> > --- a/lib/string.c
> > +++ b/lib/string.c
> > @@ -467,6 +467,9 @@ void * memcpy(void *dest, const void *src, size_t count)
> >   	unsigned long *dl = (unsigned long *)dest, *sl = (unsigned long *)src;
> >   	char *d8, *s8;
> >
> > +	if (src == dest)
> > +		return dest;
> > +
> 
> here is the same, as in the patch I've commented before. There exist no 
> reason to add a check for identity to memcpy() because memcpy doesn't 
> support overlapping regions (and identity is just a special case of 
> overlapping regions). If something might call memcpy() with overlapping 
> or identical regions, it should use memmove().

In an ideal world, nobody will ever use any interfces in a
non-compliant or incorrect way.

In reality, all kind of errors happen.  A little defensive programming
like the one above helps a lot, so please stop complaining even if you
think you don't need this.

Thanks.

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
Mirrors should reflect a little before throwing back images.
- Jean Cocteau

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23 21:55     ` Wolfgang Denk
@ 2011-05-23 22:12       ` Alexander Holler
  2011-05-23 22:22         ` Wolfgang Denk
  0 siblings, 1 reply; 22+ messages in thread
From: Alexander Holler @ 2011-05-23 22:12 UTC (permalink / raw)
  To: u-boot

Am 23.05.2011 23:55, schrieb Wolfgang Denk:
> Dear Alexander Holler,
>
> In message<4DDACC8B.6090507@ahsoftware.de>  you wrote:
>>
>>> --- a/lib/string.c
>>> +++ b/lib/string.c
>>> @@ -467,6 +467,9 @@ void * memcpy(void *dest, const void *src, size_t count)
>>>    	unsigned long *dl = (unsigned long *)dest, *sl = (unsigned long *)src;
>>>    	char *d8, *s8;
>>>
>>> +	if (src == dest)
>>> +		return dest;
>>> +
>>
>> here is the same, as in the patch I've commented before. There exist no
>> reason to add a check for identity to memcpy() because memcpy doesn't
>> support overlapping regions (and identity is just a special case of
>> overlapping regions). If something might call memcpy() with overlapping
>> or identical regions, it should use memmove().
>
> In an ideal world, nobody will ever use any interfces in a
> non-compliant or incorrect way.
>
> In reality, all kind of errors happen.  A little defensive programming
> like the one above helps a lot, so please stop complaining even if you
> think you don't need this.

So you I will look forward to checks for NULL pointers and similiar in 
all C standard functions implemented in u-boot to circumvent tons of 
possible real world bugs in all callers of strcpy, strlen, mem* and 
whatever.

Reads promising,

regards,

Alexander

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23 22:12       ` Alexander Holler
@ 2011-05-23 22:22         ` Wolfgang Denk
  2011-05-23 22:38           ` Alexander Holler
  2011-05-24 19:37           ` Scott Wood
  0 siblings, 2 replies; 22+ messages in thread
From: Wolfgang Denk @ 2011-05-23 22:22 UTC (permalink / raw)
  To: u-boot

Dear Alexander Holler,

In message <4DDADBB6.30607@ahsoftware.de> you wrote:
>
> So you I will look forward to checks for NULL pointers and similiar in 
> all C standard functions implemented in u-boot to circumvent tons of 
> possible real world bugs in all callers of strcpy, strlen, mem* and 
> whatever.

If you think a bit about this, you may find it more difficult than you
expect.  Keep in mind that on most systems supported by U-Boot code
like

	int *p = (int *)0;

	print("*p = %d\n", *p);

is perfectly legal and supposed to work without any problems -
because 0 is a legal address, and it makes perfect senze that commands
like "md" or "cp" can be used to access it.  In the result, strcpy(),
strlen(), mem*() and whatever must beable to work on address 0 likeon
any other address, too.

:-P

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
Minds are like parachutes - they only function when open.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23 22:22         ` Wolfgang Denk
@ 2011-05-23 22:38           ` Alexander Holler
  2011-05-24  3:47             ` Mike Frysinger
  2011-05-24 19:37           ` Scott Wood
  1 sibling, 1 reply; 22+ messages in thread
From: Alexander Holler @ 2011-05-23 22:38 UTC (permalink / raw)
  To: u-boot

Am 24.05.2011 00:22, schrieb Wolfgang Denk:
> Dear Alexander Holler,
>
> In message<4DDADBB6.30607@ahsoftware.de>  you wrote:
>>
>> So you I will look forward to checks for NULL pointers and similiar in
>> all C standard functions implemented in u-boot to circumvent tons of
>> possible real world bugs in all callers of strcpy, strlen, mem* and
>> whatever.
>
> If you think a bit about this, you may find it more difficult than you
> expect.  Keep in mind that on most systems supported by U-Boot code
> like
>
> 	int *p = (int *)0;
>
> 	print("*p = %d\n", *p);
>
> is perfectly legal and supposed to work without any problems -
> because 0 is a legal address, and it makes perfect senze that commands
> like "md" or "cp" can be used to access it.  In the result, strcpy(),
> strlen(), mem*() and whatever must beable to work on address 0 likeon
> any other address, too.
>
> :-P

I've never seen a valid use of strcpy() with a null-pointer in real 
world programs, which we are talking about, except in bugs.

BTW, you missed to quote my suggestion to get rid of the implementation 
of memcpy() and use always memmove(). That would be really defensive 
programming and if the unnecessary identity-check in memcpy isn't of 
interest, the additional other check done by memmove() shouldn't be a 
problem too.

But I will stop complaining as requested and getting silent again.

Regards,

Alexander

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23 22:38           ` Alexander Holler
@ 2011-05-24  3:47             ` Mike Frysinger
  2011-05-24 13:03               ` Alexander Holler
  0 siblings, 1 reply; 22+ messages in thread
From: Mike Frysinger @ 2011-05-24  3:47 UTC (permalink / raw)
  To: u-boot

On Monday, May 23, 2011 18:38:49 Alexander Holler wrote:
> Am 24.05.2011 00:22, schrieb Wolfgang Denk:
> > Alexander Holler wrote:
> >> So you I will look forward to checks for NULL pointers and similiar in
> >> all C standard functions implemented in u-boot to circumvent tons of
> >> possible real world bugs in all callers of strcpy, strlen, mem* and
> >> whatever.
> > 
> > If you think a bit about this, you may find it more difficult than you
> > expect.  Keep in mind that on most systems supported by U-Boot code
> > like
> > 
> > 	int *p = (int *)0;
> > 	
> > 	print("*p = %d\n", *p);
> > 
> > is perfectly legal and supposed to work without any problems -
> > because 0 is a legal address, and it makes perfect senze that commands
> > like "md" or "cp" can be used to access it.  In the result, strcpy(),
> > strlen(), mem*() and whatever must beable to work on address 0 likeon
> > any other address, too.
> 
> I've never seen a valid use of strcpy() with a null-pointer in real
> world programs, which we are talking about, except in bugs.

i'm lazy and type "0" all the time for loading files, copying memory, 
displaying things, etc... in u-boot.  i dont feel like retraining my finger 
muscle memory if i dont have to ;).
-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/20110523/8f2a1b98/attachment.pgp 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-24  3:47             ` Mike Frysinger
@ 2011-05-24 13:03               ` Alexander Holler
  0 siblings, 0 replies; 22+ messages in thread
From: Alexander Holler @ 2011-05-24 13:03 UTC (permalink / raw)
  To: u-boot

Am 24.05.2011 05:47, schrieb Mike Frysinger:

>> I've never seen a valid use of strcpy() with a null-pointer in real
>> world programs, which we are talking about, except in bugs.
>
> i'm lazy and type "0" all the time for loading files, copying memory,
> displaying things, etc... in u-boot.  i dont feel like retraining my finger
> muscle memory if i dont have to ;).
> -mike

Using a 4 should be better for your finger.

Alexander

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23 22:22         ` Wolfgang Denk
  2011-05-23 22:38           ` Alexander Holler
@ 2011-05-24 19:37           ` Scott Wood
  2011-05-24 20:39             ` Wolfgang Denk
  1 sibling, 1 reply; 22+ messages in thread
From: Scott Wood @ 2011-05-24 19:37 UTC (permalink / raw)
  To: u-boot

On Tue, 24 May 2011 00:22:49 +0200
Wolfgang Denk <wd@denx.de> wrote:

> Dear Alexander Holler,
> 
> In message <4DDADBB6.30607@ahsoftware.de> you wrote:
> >
> > So you I will look forward to checks for NULL pointers and similiar in 
> > all C standard functions implemented in u-boot to circumvent tons of 
> > possible real world bugs in all callers of strcpy, strlen, mem* and 
> > whatever.
> 
> If you think a bit about this, you may find it more difficult than you
> expect.  Keep in mind that on most systems supported by U-Boot code
> like
> 
> 	int *p = (int *)0;
> 
> 	print("*p = %d\n", *p);
> 
> is perfectly legal and supposed to work without any problems -

Might want to pass -fno-delete-null-pointer-checks...

As for memcpy/memmove, if in U-Boot it's to be legal to pass overlapping
regions to memcpy(), why have separate implementations (not to mention
bcopy...)?

-Scott

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-24 19:37           ` Scott Wood
@ 2011-05-24 20:39             ` Wolfgang Denk
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfgang Denk @ 2011-05-24 20:39 UTC (permalink / raw)
  To: u-boot

Dear Scott Wood,

In message <20110524143749.0b508c66@schlenkerla.am.freescale.net> you wrote:
>
> Might want to pass -fno-delete-null-pointer-checks...

Thanks for pointing out.

> As for memcpy/memmove, if in U-Boot it's to be legal to pass overlapping
> regions to memcpy(), why have separate implementations (not to mention
> bcopy...)?

Define "legal"?  Legal as in "everything that is not explicitly
forbidden"?  Legal as in "any code that does not exwecute the HCF
instruction"?

The man page says: "The memory areas should not overlap. Use
memmove(3) if the memory areas do overlap."  I have nothing to add
here.

As for the different implementations: well, U-Boot imported tons of
code from different sources, using different interfaces and library
functions.  Where needed, the missing library functions got added.

Cleanup patches are always welcome.

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
Hegel was right when he said that we learn from history that man  can
never learn anything from history.              - George Bernard Shaw

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23  9:03 ` [U-Boot] [PATCH V2] memcpy/memmove: " Matthias Weisser
  2011-05-23 21:07   ` Alexander Holler
@ 2011-06-14  6:18   ` Matthias Weißer
  2011-06-16 16:04     ` Matthias Weisser
  2011-06-30  6:31     ` Matthias Weißer
  2011-07-25 22:28   ` Wolfgang Denk
  2 siblings, 2 replies; 22+ messages in thread
From: Matthias Weißer @ 2011-06-14  6:18 UTC (permalink / raw)
  To: u-boot

Hello Wolfgang

Am 23.05.2011 11:03, schrieb Matthias Weisser:
> In some cases (e.g. bootm with a elf payload which is already at the right
> position) there is a in place copy of data to the same address. Catching this
> saves some ms while booting.

What about this patch? As the initial submission of the patch was inside 
the merge window (see http://patchwork.ozlabs.org/patch/90725/) I would 
like to see the current version of this patch (see 
http://patchwork.ozlabs.org/patch/96841/) in the upcoming release.

Sorry for the broken reference in patchwork. I try my best next time.

If the community decides to NACK the patch I would be happy if you could 
accept http://patchwork.ozlabs.org/patch/79325/

Thanks
Matthias

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-06-14  6:18   ` Matthias Weißer
@ 2011-06-16 16:04     ` Matthias Weisser
  2011-06-30  6:31     ` Matthias Weißer
  1 sibling, 0 replies; 22+ messages in thread
From: Matthias Weisser @ 2011-06-16 16:04 UTC (permalink / raw)
  To: u-boot

Am 14.06.2011 08:18, schrieb Matthias Wei?er:
> Hello Wolfgang
> 
> Am 23.05.2011 11:03, schrieb Matthias Weisser:
>> In some cases (e.g. bootm with a elf payload which is already at the right
>> position) there is a in place copy of data to the same address. Catching this
>> saves some ms while booting.
> 
> What about this patch? As the initial submission of the patch was inside 
> the merge window (see http://patchwork.ozlabs.org/patch/90725/) I would 
> like to see the current version of this patch (see 
> http://patchwork.ozlabs.org/patch/96841/) in the upcoming release.
> 
> Sorry for the broken reference in patchwork. I try my best next time.
> 
> If the community decides to NACK the patch I would be happy if you could 
> accept http://patchwork.ozlabs.org/patch/79325/

Ping?

Thanks
Matthias

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-06-14  6:18   ` Matthias Weißer
  2011-06-16 16:04     ` Matthias Weisser
@ 2011-06-30  6:31     ` Matthias Weißer
  1 sibling, 0 replies; 22+ messages in thread
From: Matthias Weißer @ 2011-06-30  6:31 UTC (permalink / raw)
  To: u-boot

Dear Wolfgang

Am 14.06.2011 08:18, schrieb Matthias Wei?er:
> Am 23.05.2011 11:03, schrieb Matthias Weisser:
>> In some cases (e.g. bootm with a elf payload which is already at the right
>> position) there is a in place copy of data to the same address. Catching this
>> saves some ms while booting.
>
> What about this patch? As the initial submission of the patch was inside
> the merge window (see http://patchwork.ozlabs.org/patch/90725/) I would
> like to see the current version of this patch (see
> http://patchwork.ozlabs.org/patch/96841/) in the upcoming release.

May I kindly ask you if http://patchwork.ozlabs.org/patch/96841/ can go 
in as the merge window is now open again?

Thanks
Matthias

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
  2011-05-23  9:03 ` [U-Boot] [PATCH V2] memcpy/memmove: " Matthias Weisser
  2011-05-23 21:07   ` Alexander Holler
  2011-06-14  6:18   ` Matthias Weißer
@ 2011-07-25 22:28   ` Wolfgang Denk
  2 siblings, 0 replies; 22+ messages in thread
From: Wolfgang Denk @ 2011-07-25 22:28 UTC (permalink / raw)
  To: u-boot

Dear Matthias Weisser,

In message <1306141435-24001-1-git-send-email-weisserm@arcor.de> you wrote:
> In some cases (e.g. bootm with a elf payload which is already at the right
> position) there is a in place copy of data to the same address. Catching this
> saves some ms while booting.
> 
> Signed-off-by: Matthias Weisser <weisserm@arcor.de>
> ---
> Changes since V1:
>   - Made subject more informative
>   - Removed the optimization from bcopy as bcopy is not used anywhere
>   
>  lib/string.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)

Applied, thanks.

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
The optimum committee has no members.
                                                   - Norman Augustine

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2011-07-25 22:28 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-12  6:58 [U-Boot] [PATCH] Do not copy to same address Matthias Weisser
2011-04-12  7:05 ` Mike Frysinger
2011-04-12  7:16   ` Matthias Weißer
2011-04-12  7:06 ` Albert ARIBAUD
2011-04-12  7:13   ` Matthias Weißer
2011-04-12  7:27     ` Albert ARIBAUD
2011-04-12  7:49       ` Matthias Weißer
2011-04-14  6:28         ` Albert ARIBAUD
2011-05-23  9:03 ` [U-Boot] [PATCH V2] memcpy/memmove: " Matthias Weisser
2011-05-23 21:07   ` Alexander Holler
2011-05-23 21:55     ` Wolfgang Denk
2011-05-23 22:12       ` Alexander Holler
2011-05-23 22:22         ` Wolfgang Denk
2011-05-23 22:38           ` Alexander Holler
2011-05-24  3:47             ` Mike Frysinger
2011-05-24 13:03               ` Alexander Holler
2011-05-24 19:37           ` Scott Wood
2011-05-24 20:39             ` Wolfgang Denk
2011-06-14  6:18   ` Matthias Weißer
2011-06-16 16:04     ` Matthias Weisser
2011-06-30  6:31     ` Matthias Weißer
2011-07-25 22:28   ` Wolfgang Denk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.