2016-03-07, 11:48:42 -0800, Stephen Hemminger wrote:
>
> > +static int one_of(const char *msg, const char *realval, const char **list,
> > + size_t len, int *index)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < len; i++) {
> > + if (matches(realval, list[i]) == 0) {
> > + *index = i;
> > + return 0;
> > + }
> > + }
> > +
> > + fprintf(stderr, "Error: argument of \"%s\" must be one of ", msg);
> > + for (i = 0; i < len; i++)
> > + fprintf(stderr, "\"%s\", ", list[i]);
> > + fprintf(stderr, "not \"%s\"\n", realval);
> > + return -1;
> > +}
> > +
> > +static int get_an(__u8 *val, const char *arg)
> > +{
> > + int ret = get_u8(val, arg, 0);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + if (*val > 3)
> > + return -1;
> > +
> > + return 0;
> > +}
> > +
> > +static int get_sci(__u64 *sci, const char *arg)
> > +{
> > + return get_u64(sci, arg, 16);
> > +}
> > +
> > +static int get_port(__be16 *port, const char *arg)
> > +{
> > + __u16 p;
> > + int ret = get_u16(&p, arg, 10);
> > +
> > + if (ret)
> > + return ret;
> > +
> > + *port = htons(p);
> > + return 0;
> > +}
>
> Some of these should probably go into lib
I could add get_be16:
int get_be16(__be16 *val, const char *arg, int base);
And maybe get_u8_range for get_an:
int get_u8_range(__u8 *val, const char *arg, int base, int start, int end);
I could also move one_of to lib, not sure if that would be used.
There are a lot of on/off in iplink, but the way I wrote one_of
doesn't really work to set/clear a flag.
Adding a function pointer to handle a match seems a bit heavy.
What do you think?
> > +static int from_hex(char c)
> > +{
> > + if (c >= '0' && c <= '9')
> > + return c - '0';
> > + if (c >= 'a' && c <= 'f')
> > + return c - 'a' + 10;
> > + if (c >= 'A' && c <= 'F')
> > + return c - 'A' + 10;
> > +
> > + return -1;
> > +}
>
> There already several copies of similar code (ipl2tp, ipmaddr, ipx)
> for basically the same code.
Right, I'll make a common helper for that.
I also noticed hexstring_a2n, which I could use if it also returned
the number of elements it parsed to the caller, so I will make that
change (it has no user for now).
> ...
> > + print_flag(stdout, attrs, "sc", MACSEC_ATTR_SC_ACTIVE);
> > + print_flag(stdout, attrs, "sa", MACSEC_ATTR_SA_ACTIVE);
> > + print_flag(stdout, attrs, "encrypt", MACSEC_ATTR_ENCRYPT);
> > + print_flag(stdout, attrs, "send_sci", MACSEC_ATTR_INC_SCI);
> > + print_flag(stdout, attrs, "end_station", MACSEC_ATTR_ES);
> > + print_flag(stdout, attrs, "scb", MACSEC_ATTR_SCB);
> > +
> > + print_flag(stdout, attrs, "replayprotect", MACSEC_ATTR_REPLAY);
> > + if (attrs[MACSEC_ATTR_WINDOW]) {
> > + printf("window %d ",
> > + rta_getattr_u32(attrs[MACSEC_ATTR_WINDOW]));
> > + }
> > +
> > + if (attrs[MACSEC_ATTR_CIPHER_SUITE] && attrs[MACSEC_ATTR_ICV_LEN]) {
> > + printf("\n");
> > + print_cipher_suite(prefix,
> > + rta_getattr_u64(attrs[MACSEC_ATTR_CIPHER_SUITE]),
> > + rta_getattr_u8(attrs[MACSEC_ATTR_ICV_LEN]));
> > + }
> > +
> > +}
>
> It is unwritten rule of ip commands that the print and set commands should
> be invertable. I.e if you are going to print an attribute the format should
> be the same as the argument passed in.
Oops, will fix.
Thanks,
--
Sabrina
