memmove broken

[Paul Mackerras]

Jörn Engel writes:

> > Why is it an issue?  Is the performance of the byte-by-byte loop
> > really a limiting factor for you?
>
> Not a limiting factor, but it should be noticable.  What is more
> important - in my eyes - is that we can replace magic with obvious
> code.

You are proposing to replace simple, working code with considerably
more complex code.  You need to be able to point to a specific real
situation where your change makes a significant difference if you want
to get it accepted.

If the behaviour of the copy loop is non-obvious then the thing to do
is to add a comment rather than make it more complicated.

> That should not be a problem.  memcpy has a very defined behaviour, as
> long as source and destination don't overlap at all or as the sourse
> is smaller, than the distination.  Cool.

The behaviour is defined as long as source and destination don't
overlap.  You get no guarantees from having source < destination.  It
could quite legitimately copy backwards in all situations, or work
exactly the same as memmove.

> We should be able to do something like this in the zlib:
> if (repeat one byte over and over) /* undefined behaviour for memcpy */
> 	memset();
> else {
> 	if (copy is wrapped) {
> 		memcpy(wrapped part);
> 		tweak pointers;
> 	}
> 	memcpy();
>
> With that, source should always be smaller than destination for
> memcpy, so the implementation details don't matter, as long as memcpy
> doesn't do a reverse memcpy.
>
> Plus, the zlib code is shorter and tells the reader exactly what it
> does, without the need for extra comments.

Well no, I think it still isn't right.  What if the length is large
but dst - src == 2 (e.g. if you compress "abababababab") or 3 (e.g. if
you compress "abcabcabcabcabc")?  Or 4 or 5 or ...?  You will end up
with an awfully large number of special cases.

Paul.

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/