On Mon, Jul 01, 2019 at 10:16:43PM +0200, Ander Juaristi wrote:
> diff --git a/include/nftables/libnftables.h b/include/nftables/libnftables.h
> index e39c588..87d4ff5 100644
> --- a/include/nftables/libnftables.h
> +++ b/include/nftables/libnftables.h
> @@ -52,6 +52,7 @@ enum {
>       NFT_CTX_OUTPUT_NUMERIC_PROTO    = (1 << 7),
>       NFT_CTX_OUTPUT_NUMERIC_PRIO     = (1 << 8),
>       NFT_CTX_OUTPUT_NUMERIC_SYMBOL   = (1 << 9),
> +     NFT_CTX_OUTPUT_SECONDS          = (1 << 10),

I'd rename this to:

        NFT_CTX_OUTPUT_NUMERIC_TIME

>       NFT_CTX_OUTPUT_NUMERIC_ALL      = (NFT_CTX_OUTPUT_NUMERIC_PROTO |
>                                          NFT_CTX_OUTPUT_NUMERIC_PRIO |
>                                          NFT_CTX_OUTPUT_NUMERIC_SYMBOL),

You have to update NFT_CTX_OUTPUT_NUMERIC_ALL.

> +static void day_type_print(const struct expr *expr, struct output_ctx *octx)
> +{
> +     const char *days[] = {
> +             "Sunday",
> +             "Monday",
> +             "Tuesday",
> +             "Wednesday",
> +             "Thursday",
> +             "Friday",
> +             "Saturday"

Probably print in short format?

        Sun
        Mon
        Tue
        Wed
        Thu
        Fri
        Sat

like 'date'.

> +     };
> +     uint8_t daynum = mpz_get_uint8(expr->value),

missing line break between variable definition and code.

> +              numdays = array_size(days);
        ^^^^^^^^^

Unnecessary indentation.

> +
> +     if (daynum >= 0 && daynum < numdays)
> +             nft_print(octx, "\"%s\"", days[daynum]);
> +     else
> +             nft_print(octx, "Unknown day");
> +}
> +
> +static int parse_day_type_numeric(const char *sym)
> +{
> +     char c = *sym;
> +     return (c >= '0' && c <= '6') ?
> +             (c - '0') :
> +             -1;

Oh, I'd suggest:

        if (c >= '0' && c <= '6')
                return (c - '0');

        return -1;

> +}
> +
> +static struct error_record *day_type_parse(const struct expr *sym,
> +                                        struct expr **res)
> +{
> +     const char *days[] = {
> +             "Sunday",
> +             "Monday",
> +             "Tuesday",
> +             "Wednesday",
> +             "Thursday",
> +             "Friday",
> +             "Saturday"
> +     };

Can you add a helper function to translate numeric day to literal? So
you don't need to define this again, probably:

        day_str = nft_day_str(day_num);

> +     int daynum = -1, numdays = array_size(days);
> +     int symlen = strlen(sym->identifier), daylen;
> +
> +     if (symlen < 3) {
> +             if (symlen == 1)
> +                     daynum = parse_day_type_numeric(sym->identifier);
> +
> +             if (daynum >= 0)
> +                     goto success;

goto for success path is discouraged.

Better if you use 'goto' to deal with error path.

> +
> +             return error(&sym->location, "Day name must be at least three 
> characters long");
> +     }
> +
> +     for (int i = 0; i < numdays && daynum == -1; i++) {
> +             daylen = strlen(days[i]);
> +
> +             if (strncasecmp(sym->identifier,
> +                             days[i],
> +                             min(symlen, daylen)) == 0)
> +                     daynum = i;
> +     }

Introduce a helper function to do this literal to numeric lookup?

Reply via email to