Re: [Lynx-dev] ..on SOCKS5 support

[Mouse]

>> replacement you propose "should ... be optimisable",
> that was not the reasonâ?? the code I proposed to be replaced was
> unportable and just in general bad style.

Here's the diff I saw.

-       /* C99 */  {
-           unsigned short x;   /* XXX 16-bit? */
-
-           x = htons(socks5_port);
-           memcpy(&pbuf[i], (unsigned char *) &x, sizeof x);
-           i += (unsigned) sizeof(x);
-       }
+       pbuf[i++] = (((unsigned)socks5_port) >> 8) & 0xFF;
+       pbuf[i++] = ((unsigned)socks5_port) & 0xFF;

What systems are you aware of where your replacement works but the
original fails?  (If none, then in what sense is the one more portable
than the other?)

The original has portability issues, yes - it assumes an unsigned short
containing an htons() return value has the same size and memory layout
as what it's trying to generate.  (It also assumes htons and memcpy
exist.  memcpy is part of C99 in hosted implementations; I suppose one
could claim assumption of the existence of memcpy as a portability
issue, though I wouldn't, not for something that assumes the existence
of SOCKS5.  Similarly, one could claim assuming the presence of htons()
as a portability issue; again, for something already depending on
SOCKS5, I wouldn't.)  As for whether what it generates is what it needs
to generate, I don't know - for that, I'd need to see the relevant
interface spec, which isn't given.

Your replacement also has portability issues, to be sure, though there
are fewer of them and they are less likely to fail.  (The major ones I
see offhand are that it assumes 8-bit units in the &0xFF operations and
it makes the assumption that two elements of pbuf[] holding 8-bit units
in big-endian order is correct for what it's trying to generate; again,
to tell how correct this is I'd need to see interface contracts.)

As for bad style, I don't see anything particularly wrong with the old
code's style, possibly excepting disagreeing with the surrounding
code's style.  (There are various things I don't like about its style,
but "style I don't like" != "bad style".)

> I meant optimise to whatever happens to be nice on the target
> machine, of course (and if two separate stores are, so be it),

Yes, of course - but this isn't FORTRAN; there is significantly less
room than one might wish for compilers to optimize in C.  I'd actually
be surprised, though only a little, to find a C compiler that would
optimize those two assignment statements into a single 16-bit store
even on a big-endian alignment-doesn't-matter machine.

/~\ The ASCII                             Mouse
\ / Ribbon Campaign
 X  Against HTML                address@hidden
/ \ Email!           7D C8 61 52 5D E7 2D 39  4E F1 31 3E E8 B3 27 4B