Re: [PATCH 2/7] dpwsockx: Implementation of SPInit

[Ismael Barros²]

On Fri, Sep 4, 2009 at 12:12 PM, Henri Verbeet wrote:
> 2009/9/4 Ismael Barros² :
> > I've been looking into iphlpapi/ip.h (just learned bit fields
> > exist...); would this implementation be fine?
> >
> > #include "pshpack1.h"
> >
> > typedef struct tagDPSP_MSG_HEADER
> > {
> > #ifdef WORDS_BIGENDIAN
> >    DWORD size:20;
> >    DWORD token:12;
> > #else
> >    DWORD token:12;
> >    DWORD size:20;
> > #endif
> >    SOCKADDR_IN SockAddr;
> > } DPSP_MSG_HEADER, *LPDPSP_MSG_HEADER;
>
> Probably not. I'm not sure iphlpapi/ip.h does things right in the
> first place, it seems to confuse bitfield ordering with machine byte
> ordering. As a general rule, I think it's best to avoid bitfields for
> things like writing data to a file or sending data over the network,
> it's just a pain. Aside from that, using the bitfields in this way
> messes with the bit ordering, but doesn't change the byte order. For
> the bitfields in iphlpapi/ip.h that's not an issue because they fit in
> a single byte. Just store the data in a DWORD with the appropriate
> masks and shifts, and byte swap that DWORD when reading/writing it.

Here is the next try, how does it look?
It seems to behave correctly in some quick tests (byte-swapping
parameters to emulate big-endian).

Thanks a lot for the feedback, really appreciated.

-------------

#ifdef WORDS_BIGENDIAN

static inline u_short __dpws_ushort_swap(u_short s)
{
    return (s >> 8) | (s << 8);
}
static inline u_long __dpws_ulong_swap(u_long l)
{
    return ((u_long)__dpws_ushort_swap((u_short)l) << 16) |
__dpws_ushort_swap((u_short)(l >> 16));
}

#define dpws_letohl(l) __dpws_ulong_swap(l)
#define dpws_letohs(s) __dpws_ushort_swap(s)
#define dpws_htolel(l) __dpws_ulong_swap(l)
#define dpws_htoles(s) __dpws_ushort_swap(s)

#else /* WORDS_BIGENDIAN */

#define dpws_letohl(l) ((u_long)(l))
#define dpws_letohs(s) ((u_short)(s))
#define dpws_htolel(l) ((u_long)(l))
#define dpws_htoles(s) ((u_short)(s))

#endif /* WORDS_BIGENDIAN */

#include "pshpack1.h"

typedef struct tagDPSP_MSG_HEADER
{
    DWORD       mixed;
    SOCKADDR_IN SockAddr;
} DPSP_MSG_HEADER, *LPDPSP_MSG_HEADER;
typedef const DPSP_MSG_HEADER* LPCDPSP_MSG_HEADER;

#include "poppack.h"

#define DPSP_MSG_TOKEN_REMOTE    0xFAB00000
#define DPSP_MSG_TOKEN_FORWARDED 0xCAB00000
#define DPSP_MSG_TOKEN_SERVER    0xBAB00000

#define DPSP_MSG_MAKE_MIXED(s,t) dpws_htolel((s) | (t))
#define DPSP_MSG_SIZE(m)         dpws_letohl((m) & 0x000FFFFF)
#define DPSP_MSG_TOKEN(m)        ((m) & 0xFFF00000)