On Wed, Jan 23, 2019 at 12:15 AM Bart Van Assche <bvanass...@acm.org> wrote:
> On 1/22/19 12:40 PM, Bill Fenner wrote: > > On Wed, Nov 7, 2018 at 1:26 AM Bart Van Assche <bvanass...@acm.org > > <mailto:bvanass...@acm.org>> wrote: > > > > On 11/6/18 8:03 AM, Bill Fenner wrote: > > > Given your proposed code structure, I imagine that we could add > > network > > > namespaces to netsnmp_ep too - this basically ends up using > > "socketat( > > > /* magic */, family, type, protocol )" instead of socket() to > > create the > > > socket, and the magic can be derived from what we store in ep. > > > > That sounds like a good idea to me. > > > > > > I've finally done this, and would like to request some eyes on it before > > I push it to 5-8-patches. > > > > > https://github.com/fenner/net-snmp/compare/V5-8-patches...fenner:linux-namespace > > Hi Bill, > > The following new code in netsnmp_parse_ep_str() looks a bit fragile to me: > > cp = strchr(iface, '@'); > if (!cp) > cp = strchr(iface, ':'); > > Wouldn't it be better to backtrack to the previous value of 'cp' if no > second '@' sign is found? That will avoid that this code fails if > support for a new delimiter between '@' and ':' would be added. > The pattern I was trying to follow from the existing code appeared to be basically if (cp == delimiter of optional section) { *cp = '\0'; /* terminate previous section at delimiter */ this = cp + 1; /* handle this section */ cp = ... /* find next optional delimiter or NULL */ } Are you suggesting instead to put the "find next optional delimiter" code between sections, e.g., /* parse IP address, cp still points at beginning of address */ maybeintf = strchr( cp, '@' ); if (maybeintf) { *maybeintf = '\0'; cp = maybeintf + 1; intf = cp; } maybens = strchr( cp, '@' ); if (maybens) { ... } maybeport = strchr( cp, ':' ); if (maybeport) { ... } where at the end of each block, cp still points to the beginning of the thing that was parsed by the block (address, intf, namespace, or port) and we advance the pointer between blocks? The following code looks unusual to me: > > #ifdef HAVE_SETNS > #include <sys/types.h> > #include <sys/stat.h> > #include <fcntl.h> > #include <sys/socket.h> > #include <unistd.h> > #include <signal.h> > #include <sched.h> > #include <net-snmp/library/snmp_assert.h> > #endif > > I'm not aware of any other Net-SNMP code that guards include directives > with the result of a test for a system call. > Well, I was just trying to reduce the #include footprint to match the new code - if the new code in netsnmp_socketat() isn't compiled, then the new #includes that it requires aren't required. Maybe this is a leftover habit from learning C in the 1980's :-) Bill
_______________________________________________ Net-snmp-coders mailing list Net-snmp-coders@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/net-snmp-coders
