[nanomsg] Re: [Non-DoD Source] Minor forthcoming NNG change, may break programs

  • From: "Garrett D'Amore" <garrett@xxxxxxxxxx>
  • To: nanomsg@xxxxxxxxxxxxx
  • Date: Fri, 04 May 2018 14:55:28 +0000

nng guarantees that socket ids are all in the range [1 , 0x7FFFFFFF]. This
promise is a result of the idhash code used that those ids are indices
into.

nng requires at least a 32-bit processor with 32-bit ints (or 64bits). It
also requires that longs and pointers are no larger than 64 bits.

I am not interested in trying to support platforms that are 16-bit. Such
systems will generally require special measures to write code for and not
support underlying POSIX sockets, multitasking, or similar.
On Fri, May 4, 2018 at 6:01 AM Karan, Cem F CIV USARMY RDECOM ARL (US) <
cem.f.karan.civ@xxxxxxxx> wrote:

Looking at the signature for nng_socket_id() at
https://github.com/nanomsg/nng/blob/master/src/nng.h:

typedef struct nng_socket_s {
        uint32_t id;
} nng_socket;

NNG_DECL int nng_socket_id(nng_socket);

This also makes my spidey sense tingle; what is the guaranteed upper limit
on the number of sockets that can be opened at one time?  Are there any
special socket IDs, or ranges of socket IDs, that can't be used or passed
out? What is the guaranteed minimum size for an int (as far as nng is
concerned)?  The last question is important when compiling on embedded
systems.

My worry is that there will be enough live sockets out there that they
overflow what can be returned in the non-negative portion that the int can
handle.  At that point what should be valid sockets will start to appear to
be invalid ones.  I'd suggest using the following API:

// Returns true iff the socket is valid.
NNG_DECL bool nng_socket_is_valid(nng_socket);

// Comparison function compatible with the C stdlib qsort() and bsearch()
functions.
// Each argument will be cast as a pointer to a nng_socket instance.  Note
that this
// function never fails; in particular, even if nng_socket_is_valid()
returns false for
// any argument to this function, this function will still perform the
comparison as if
// the argument were valid.  You are responsible for either ensuring that
all arguments
// are valid  before calling this function, or filtering out invalid
arguments afterwards.
NNG_DECL int nng_socket_compare(const void*,const  void*);

Thanks,
Cem Karan

-----Original Message-----
From: nanomsg-bounce@xxxxxxxxxxxxx [mailto:nanomsg-bounce@xxxxxxxxxxxxx]
On Behalf Of Garrett D'Amore
Sent: Thursday, May 3, 2018 5:53 PM
To: nanomsg@xxxxxxxxxxxxx
Subject: [nanomsg] Re: [Non-DoD Source] Minor forthcoming NNG change, may
break programs

All active links contained in this email were disabled. Please verify the
identity of the sender, and confirm the authenticity of all links contained
within the message prior to copying and pasting the address to a Web
browser.


________________________________



Actually, I went down a different direction.  nng_socket_id() and related
functions.  These all return a positive value for a good socket, and -1
otherwise.  Zero initialization is treated as invalid, and causes these
functions to return -1.  There are some initializer macros now too —
NNG_SOCKET_INITIALIZER etc.  Much like POSIX mutexes and similar.

  - Garrett

On Mon, Apr 30, 2018 at 5:52 AM Karan, Cem F CIV USARMY RDECOM ARL (US) <
cem.f.karan.civ@xxxxxxxx < Caution-mailto:cem.f.karan.civ@xxxxxxxx ;> >
wrote:


        To avoid future problems, would you be willing to add in
comparison() and is_null() functions?  I just have this bad feeling that
using memcmp() could cause issues in future code (no real idea what, more
like a spidey-sense of badness). Having a function that does comparisons
that we use would make it easy to fix things if needed.

        Thanks,
        Cem Karan

        -----Original Message-----
        From: nanomsg-bounce@xxxxxxxxxxxxx < Caution-mailto:
nanomsg-bounce@xxxxxxxxxxxxx >  [Caution-mailto:
nanomsg-bounce@xxxxxxxxxxxxx < Caution-mailto:nanomsg-bounce@xxxxxxxxxxxxx
] On Behalf Of Garrett D'Amore
        Sent: Thursday, April 26, 2018 7:40 PM
        To: nanomsg@xxxxxxxxxxxxx < Caution-mailto:nanomsg@xxxxxxxxxxxxx

        Subject: [Non-DoD Source] [nanomsg] Minor forthcoming NNG change,
may break programs

        I’ve been bitten too many times by C’s crummy support for strong
type checks of integer typedefs, where I pass an  nng_socket type into an
nng_ctx or some other confusion.   And if I’m struggling with this, I think
others will to.  So I’ve decided to fix it.

        The fix is that these types, which are currently all typedef’d to
uint32_t, will instead become structures containing a single member — like
this:

        typedef struct { uint32_t id; } nng_socket;

        This value can be passed around on the stack and through return
values with no extra overhead (at least if the optimizer is turned on).

        The reason this may break your programs is that the following will
hold true:

        a) Comparison of sockets will now need to be done either by
accessing the id field (which I’d rather you didn’t do) or memcmp.  (There
is little reason normally to compare these.)

        b) Checking for a “null” value (zero) means accessing the member
field.  Again this is not something normal programs should need to do.

        c) If you’ve actually cast these things into integer types in the
past, that won’t work.  (That was always a bad idea.)

        Note that assignments, passing through as is is all unaffected.
The vast majority of user code should be entirely unaffected.  The huge
benefit is that the compiler now tells you when you mix up argument types
or pass the wrong type to the wrong function.  The savings in confusion has
the potential to be enormous.

        Please let me know if you have any concerns.  I’ve made the code
changes already, and once I finish updating the documentation to reflect
this I expect to merge this back into master for NNG.

        Thanks.

         - Garrett



Other related posts: