On Wed, Jul 05, 2017 at 05:27:18PM +0200, Ingo Schwarze wrote:
Hi Klemens,
Klemens Nanni wrote on Wed, Jul 05, 2017 at 05:02:05PM +0200:
No need for buffers t0, t1 here.
Your patch changes behaviour in some cases where the buffers do
overlap. For example, if src == dst, right now, the code swaps
bytes. With your patch, i'm not sure it is even deterministic
any longer, and whatever it does exactly, it definitely destroys
information.
But the restrict keyword tells the compiler to expect distinct addresses
and POSIX explicitly says that overlapping regions cause undefined
behaviour.
So I'd say for cases like src == dst we don't have to guarantee that
bytes are swapped.
Even if behaviour is explicitly unspecified, changing it still
requires a rationale and a careful assessment of potential
damage to existing software, even if software may only be
affected when carelessly written.
Agreed, I haven't checked for bad/dangerous usage in existing code for
reasons explained above.
POSIX leaves treatment of the last odd byte unspecified but we
don't touch it at all so why not documenting this behaviour?
I strongly oppose your patch to the manual page.
Documenting implementation details is not among the goals of
manual pages, in particular when they are not portable and should
not be relied upon.
Documenting what is specified, and what is not, is among the
goals of manual pages.
In that sense, your patch to the manual page introduces a bug and
turns the STANDARDS section into a lie.
Point taken, as I already mentioned I wasn't sure about the manual diff.
No need to fix it because the patch is not likely to go anywhere,
but once again you mangled the patch such that it won't even apply.
Hm, the diff taken from my mail as is applies cleanly here.
