Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
Dear Matthias Weisser, In message <1306141435-24001-1-git-send-email-weiss...@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 > --- > 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: w...@denx.de The optimum committee has no members. - Norman Augustine ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
Dear Scott Wood, In message <20110524143749.0b508...@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: w...@denx.de Hegel was right when he said that we learn from history that man can never learn anything from history. - George Bernard Shaw ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
On Tue, 24 May 2011 00:22:49 +0200
Wolfgang Denk wrote:
> Dear Alexander Holler,
>
> In message <4ddadbb6.30...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
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
signature.asc
Description: This is a digitally signed message part.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
Am 24.05.2011 00:22, schrieb Wolfgang Denk:
> Dear Alexander Holler,
>
> In message<4ddadbb6.30...@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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
Dear Alexander Holler,
In message <4ddadbb6.30...@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: w...@denx.de
Minds are like parachutes - they only function when open.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
Am 23.05.2011 23:55, schrieb Wolfgang Denk: > Dear Alexander Holler, > > In message<4ddacc8b.6090...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
Dear Alexander Holler, In message <4ddacc8b.6090...@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: w...@denx.de Mirrors should reflect a little before throwing back images. - Jean Cocteau ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
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
> ---
> 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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH V2] memcpy/memmove: Do not copy to same address
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
---
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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
