[ofa-general] Re: [PATCH] ibsim/sim.h: Fix NodeDescription size
Hal Rosenstock
hrosenstock at xsigo.com
Mon May 12 12:20:25 PDT 2008
On Mon, 2008-05-12 at 21:25 +0000, Sasha Khapyorsky wrote:
> On 09:28 Mon 12 May , Hal Rosenstock wrote:
> >
> > I made NODEIDLEN 65 rather than 64 due to the +1 in the original define.
> > How about defining it as 64 as in the below ? Does that get around the
> > overflow issue ?
> >
> > -- Hal
> >
> > ibsim/sim.h: Fix NodeDescription size so can have maximum size
> > NodeDescription per IBA spec rather than truncating them
> >
> > Signed-off-by: Hal Rosenstock <hal at xsigo.com>
> >
> > diff --git a/ibsim/sim.h b/ibsim/sim.h
> > index bea136a..0bf14fd 100644
> > --- a/ibsim/sim.h
> > +++ b/ibsim/sim.h
> > @@ -65,9 +65,8 @@
> > #define DEFAULT_LINKWIDTH LINKWIDTH_4x
> > #define DEFAULT_LINKSPEED LINKSPEED_SDR
> >
> > -#define NODEIDBASE 20
> > #define NODEPREFIX 20
I think this can now be eliminated as the only use was in NODEIDLEN.
> > -#define NODEIDLEN (NODEIDBASE+NODEPREFIX+1)
> > +#define NODEIDLEN 64
> > #define ALIASLEN 40
>
> It is likely will prevent overflow, but will potentially truncate last
> NodeDesc character due to string NULL terminator. What about something
> like below?
Thanks. This seems to work for my usage but not sure about some of the
other concatenated names and whether it accomodates those other usages
which I don't fully understand.
-- Hal
> Sasha
>
>
> diff --git a/ibsim/sim.h b/ibsim/sim.h
> index 81bb47c..d3294f4 100644
> --- a/ibsim/sim.h
> +++ b/ibsim/sim.h
> @@ -67,7 +67,7 @@
>
> #define NODEIDBASE 20
> #define NODEPREFIX 20
> -#define NODEIDLEN (NODEIDBASE+NODEPREFIX+1)
> +#define NODEIDLEN 65
> #define ALIASLEN 40
>
> #define MAXHOPS 16
> @@ -237,7 +237,7 @@ struct Node {
> uint64_t sysguid;
> uint64_t nodeguid; // also portguid
> int portsbase; // in port table
> - char nodeid[64]; // contain nodeid[NODEIDLEN]
> + char nodeid[NODEIDLEN]; // contain nodeid[NODEIDLEN]
> uint8_t nodeinfo[64];
> char nodedesc[64];
> Switch *sw;
> diff --git a/ibsim/sim_cmd.c b/ibsim/sim_cmd.c
> index 5f64229..fe3e9be 100644
> --- a/ibsim/sim_cmd.c
> +++ b/ibsim/sim_cmd.c
> @@ -56,7 +56,7 @@ extern Port *ports;
> extern Port **lids;
> extern int netnodes, netports, netswitches;
>
> -#define NAMELEN 64
> +#define NAMELEN NODEIDLEN
>
> char *portstates[] = {
> "-", "Down", "Init", "Armed", "Active",
> diff --git a/ibsim/sim_net.c b/ibsim/sim_net.c
> index bf7a06a..2a9c19b 100644
> --- a/ibsim/sim_net.c
> +++ b/ibsim/sim_net.c
> @@ -267,17 +267,16 @@ static int new_hca(Node * nd)
> return 0;
> }
>
> -static int build_nodeid(char *nodeid, char *base)
> +static int build_nodeid(char *nodeid, size_t len, char *base)
> {
> if (strchr(base, '#') || strchr(base, '@')) {
> IBWARN("bad nodeid \"%s\": '#' & '@' characters are reserved",
> base);
> return -1;
> }
> - if (netprefix[0] == 0)
> - strncpy(nodeid, base, NODEIDLEN);
> - else
> - snprintf(nodeid, NODEIDLEN, "%s#%s", netprefix, base);
> +
> + snprintf(nodeid, len, "%s%s%s", netprefix, *netprefix ? "#" : "", base);
> +
> return 0;
> }
>
> @@ -287,7 +286,7 @@ static Node *new_node(int type, char *nodename, char *nodedesc, int nodeports)
> char nodeid[NODEIDLEN];
> Node *nd;
>
> - if (build_nodeid(nodeid, nodename) < 0)
> + if (build_nodeid(nodeid, sizeof(nodeid), nodename) < 0)
> return 0;
>
> if (find_node(nodeid)) {
> @@ -310,11 +309,9 @@ static Node *new_node(int type, char *nodename, char *nodedesc, int nodeports)
>
> nd->type = type;
> nd->numports = nodeports;
> - strncpy(nd->nodeid, nodeid, NODEIDLEN - 1);
> - if (nodedesc && nodedesc[0])
> - strncpy(nd->nodedesc, nodedesc, NODEIDLEN - 1);
> - else
> - strncpy(nd->nodedesc, nodeid, NODEIDLEN - 1);
> + strncpy(nd->nodeid, nodeid, sizeof(nd->nodeid) - 1);
> + strncpy(nd->nodedesc, nodedesc && *nodedesc ? nodedesc : nodeid,
> + sizeof(nd->nodedesc) - 1);
> nd->sysguid = nd->nodeguid = guids[type];
> if (type == SWITCH_NODE) {
> nodeports++; // port 0 is SMA
> @@ -551,22 +548,20 @@ char *expand_name(char *base, char *name, char **portstr)
> if (netprefix[0] != 0 && !strchr(base, '#'))
> snprintf(name, NODEIDLEN, "%s#%s", netprefix, base);
> else
> - strcpy(name, base);
> + strncpy(name, base, NODEIDLEN - 1);
> if (portstr)
> - *portstr = 0;
> + *portstr = NULL;
> PDEBUG("name %s port %s", name, portstr ? *portstr : 0);
> return name;
> }
> - if (base[0] == '@')
> - snprintf(name, ALIASLEN, "%s%s", netprefix, base);
> - else
> - strcpy(name, base);
> +
> + snprintf(name, NODEIDLEN, "%s%s", base[0] == '@' ? netprefix : "", base);
> PDEBUG("alias %s", name);
>
> if (!(s = map_alias(name)))
> return 0;
>
> - strcpy(name, s);
> + strncpy(name, s, NODEIDLEN - 1);
>
> if (portstr) {
> *portstr = name;
> @@ -1075,12 +1070,12 @@ int link_ports(Port * lport, Port * rport)
> lport->remotenode = rnode;
> lport->remoteport = rport->portnum;
> set_portinfo(lport, lnode->type == SWITCH_NODE ? swport : hcaport);
> - memcpy(lport->remotenodeid, rnode->nodeid, NODEIDLEN);
> + memcpy(lport->remotenodeid, rnode->nodeid, sizeof(lport->remotenodeid));
>
> rport->remotenode = lnode;
> rport->remoteport = lport->portnum;
> set_portinfo(rport, rnode->type == SWITCH_NODE ? swport : hcaport);
> - memcpy(rport->remotenodeid, lnode->nodeid, NODEIDLEN);
> + memcpy(rport->remotenodeid, lnode->nodeid, sizeof(rport->remotenodeid));
> lport->state = rport->state = 2; // Initialilze
> lport->physstate = rport->physstate = 5; // LinkUP
> if (lnode->sw)
> @@ -1166,7 +1161,7 @@ int connect_ports(void)
> }
> } else if (remoteport->remoteport != port->portnum ||
> strncmp(remoteport->remotenodeid, port->node->nodeid,
> - NODEIDLEN)) {
> + sizeof(remoteport->remotenodeid))) {
> IBWARN
> ("remote port %d in node \"%s\" is not connected to "
> "node \"%s\" port %d (\"%s\" %d)",
> _______________________________________________
> general mailing list
> general at lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
More information about the general
mailing list