Hi David,
On 07/06/2018 08:58 AM, David Ahern wrote:
> On 7/5/18 4:42 PM, Jesus Sanchez-Palencia wrote:
>
>> +static int get_clockid(__s32 *val, const char *arg)
>> +{
>> + const struct static_clockid {
>> + const char *name;
>> + clockid_t clockid;
>> + } clockids_sysv[] = {
>> + { "CLOCK_REALTIME", CLOCK_REALTIME },
>> + { "CLOCK_TAI", CLOCK_TAI },
>> + { "CLOCK_BOOTTIME", CLOCK_BOOTTIME },
>> + { "CLOCK_MONOTONIC", CLOCK_MONOTONIC },
>> + { NULL }
>> + };
>> +
>> + const struct static_clockid *c;
>> +
>> + for (c = clockids_sysv; c->name; c++) {
>> + if (strncasecmp(c->name, arg, 25) == 0) {
>
> Why 25?
That was just an upper bound giving some room beyond the longest
clockid name we have today. Should I add a define MAX_CLOCK_NAME ?
>
> be nice to allow shortcuts -- e.g., just REALTIME or realtime.
I'd rather just keep it as is and use the names as they are defined for
everything else (i.e. CLOCK_REALTIME), unless there are some strong objections.
>
>> + *val = c->clockid;
>> +
>> + return 0;
>> + }
>> + }
>> +
>> + return -1;
>> +}
>> +
>> +
>> +static int etf_parse_opt(struct qdisc_util *qu, int argc,
>> + char **argv, struct nlmsghdr *n, const char *dev)
>> +{
>> + struct tc_etf_qopt opt = {
>> + .clockid = CLOCKID_INVALID,
>> + };
>> + struct rtattr *tail;
>> +
>> + while (argc > 0) {
>> + if (matches(*argv, "offload") == 0) {
>> + if (opt.flags & TC_ETF_OFFLOAD_ON) {
>> + fprintf(stderr, "etf: duplicate \"offload\"
>> specification\n");
>> + return -1;
>> + }
>> +
>> + opt.flags |= TC_ETF_OFFLOAD_ON;
>> + } else if (matches(*argv, "deadline_mode") == 0) {
>> + if (opt.flags & TC_ETF_DEADLINE_MODE_ON) {
>> + fprintf(stderr, "etf: duplicate
>> \"deadline_mode\" specification\n");
>> + return -1;
>> + }
>> +
>> + opt.flags |= TC_ETF_DEADLINE_MODE_ON;
>> + } else if (matches(*argv, "delta") == 0) {
>> + NEXT_ARG();
>> + if (opt.delta) {
>> + fprintf(stderr, "etf: duplicate \"delta\"
>> specification\n");
>> + return -1;
>> + }
>> + if (get_s32(&opt.delta, *argv, 0)) {
>> + explain1("delta", *argv);
>> + return -1;
>> + }
>> + } else if (matches(*argv, "clockid") == 0) {
>> + NEXT_ARG();
>> + if (opt.clockid != CLOCKID_INVALID) {
>> + fprintf(stderr, "etf: duplicate \"clockid\"
>> specification\n");
>> + return -1;
>> + }
>> + if (get_clockid(&opt.clockid, *argv)) {
>> + explain_clockid(*argv);
>> + return -1;
>> + }
>> + } else if (strcmp(*argv, "help") == 0) {
>> + explain();
>> + return -1;
>> + } else {
>> + fprintf(stderr, "etf: unknown parameter \"%s\"\n",
>> *argv);
>> + explain();
>> + return -1;
>> + }
>> + argc--; argv++;
>> + }
>> +
>> + tail = NLMSG_TAIL(n);
>> + addattr_l(n, 1024, TCA_OPTIONS, NULL, 0);
>> + addattr_l(n, 2024, TCA_ETF_PARMS, &opt, sizeof(opt));
>> + tail->rta_len = (void *) NLMSG_TAIL(n) - (void *) tail;
>> + return 0;
>> +}
>> +
>> +static int etf_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
>> +{
>> + struct rtattr *tb[TCA_ETF_MAX+1];
>> + struct tc_etf_qopt *qopt;
>> +
>> + if (opt == NULL)
>> + return 0;
>> +
>> + parse_rtattr_nested(tb, TCA_ETF_MAX, opt);
>> +
>> + if (tb[TCA_ETF_PARMS] == NULL)
>> + return -1;
>> +
>> + qopt = RTA_DATA(tb[TCA_ETF_PARMS]);
>> + if (RTA_PAYLOAD(tb[TCA_ETF_PARMS]) < sizeof(*qopt))
>> + return -1;
>> +
>> + if (qopt->clockid == CLOCKID_INVALID)
>> + print_string(PRINT_ANY, "clockid", "clockid %s ", "invalid");
>> + else
>> + print_uint(PRINT_ANY, "clockid", "clockid %d ", qopt->clockid);
>
> If you allow the user to input a string, then you should return it here too.
Ok, will fix.
Thanks,
Jesus
