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.
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.
> 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.
> Feedback? Comments?
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.
Yours,
Ingo
> Index: swab.c
> ===================================================================
> RCS file: /cvs/src/lib/libc/string/swab.c,v
> retrieving revision 1.9
> diff -u -p -r1.9 swab.c
> --- swab.c 11 Dec 2014 23:05:38 -0000 1.9
> +++ swab.c 5 Jul 2017 14:12:34 -0000
> @@ -21,15 +21,8 @@ swab(const void *__restrict from, void *
> {
> const unsigned char *src = from;
> unsigned char *dst = to;
> - unsigned char t0, t1;
> + ssize_t i;
>
> - while (len > 1) {
> - t0 = src[0];
> - t1 = src[1];
> - dst[0] = t1;
> - dst[1] = t0;
> - src += 2;
> - dst += 2;
> - len -= 2;
> - }
> + for (i = 0; i < (len & ~1) - 1; i += 2)
> + dst[i] = src[i + 1], dst[i + 1] = src[i];
> }
> Index: swab.3
> ===================================================================
> RCS file: /cvs/src/lib/libc/string/swab.3,v
> retrieving revision 1.9
> diff -u -p -r1.9 swab.3
> --- swab.3 12 Dec 2014 20:06:13 -0000 1.9
> +++ swab.3 5 Jul 2017 14:12:34 -0000
> @@ -57,7 +57,9 @@ If
> is zero or less,
> .Nm
> does nothing.
> -If it is odd, what happens to the last byte is unspecified.
> +If it is odd,
> +.Fa len Ns \-1
> +bytes are swapped and the last byte is ignored.
> If
> .Fa src
> and
