On Wed, Dec 19, 2007 at 08:51:08PM +0100, Andreas Henriksson wrote:
> Here's an additional patch on top of the previous in the series. It
> restores backwards compatability for the "hwaddress" option (and drops
> the new lladdress option that was invented for the new syntax).

Ah, I don't think I can accept that one; the philosophy is to keep the
ifupdown proper ignorant of what commands are actually used, and have
all the specific knowledge in the OS-specific appendix.

However, I don't see why the same thing can't be done with a hook; so
that you add a function to archlinux.c, like:

void cleanup_hwaddress_for_iproute(char **pparam) {
        char *rest = *pparam; /* we're shrinking the text,
                               * so no realloc needed */
> +     char *space = strchr(rest, ' ');
> +             
> +     if (space != NULL) {
> +             *space = '\0';
> +             if (strcasecmp(rest, "ether") == 0 ||
> +                     strcasecmp(rest, "ax25") == 0 ||
> +                     strcasecmp(rest, "ARCnet") == 0 ||
> +                     strcasecmp(rest, "netrom") == 0)
> +             {
> +                     /* found deprecated <class> attribute */
> +                     memmove(rest, space+1, strlen(space+1)+1);
> +             }
> +             else
> +             {
> +                     *space = ' ';
> +             }
> +     }
}

Add a note to <<inet methords>> something like:

method static
  description
    This method may be used to define ethernet interfaces with statically
    allocated IPv4 addresses.
      
  options
    ...

  conversion
    hwaddress cleanup_hwaddress_for_iproute

  up
    ...
  down
    ...

Add a "conversion" field to struct method that points to an array of
{char *, void (*)(char **)} entries, terminated by {NULL,NULL}.

(Alternatively, and more flexibly, the function could be
void(*)(interface_defn *), which would allow a conversion function to
create a full dotted-quad netmask given an address like 192.168.1.2/24,
and also remove the /24 from the address itself)

Do the necessary magic in defn2c.pl to get the above translated into C code
that creates the appropriate array of conversions, and links it all.

At that point we're back to:

>  <<convert [[post-up]] and [[pre-down]] aliases to [[up]] and [[down]]>>
>  <<check for duplicate options>>
>  <<add option>>

And basically want to add a <<convert option>> after adding the option
that does something like:

<<convert option>>=
{
        /* assumes:
         * struct conversion { char *option; void(*fn)(char**); };
         * typedef struct conversion conversion;
         * and interfaces_defn has a "conversion *conversion;" field
         */
        conversion *c;
        variable *o = currif->option[n_options-1];
        for (c = currif->method->conversions; c->option && c->fn; c++) {
                if (strcmp(c->option, o->name) == 0) {
                        c->fn(&o->value);
                        break;
                }
        }
}
@

Alternatively it might be better (and maybe simpler) to have one
"conversion" function per method and only call it when the interface_defn
has been completely read, which would mean just looping over all the
interfaces at the end of read_interfaces():

typedef void(*conversionfn)(variable **option, int *n_opt, int *max_opt);

<<update interface definitions where necessary>>=
for (currif = defn->ifaces; currif; currif = currif->next) {
        if (currif->method->conversion) {
                currif->method->conversion(
                        &currif->option,
                        &currif->n_options,
                        &currif->max_options
                );
        }
}
@

That lets you generate fake parameters however you see fit, like
converting "netmask 255.255.255.0" to a new option "cidr 24", so that
you can then tell iproute %address%/%cidr%, which seems like the right
way to go to get non-Linux stuff working right too to me.

What do you think?

Cheers,
aj

Attachment: signature.asc
Description: Digital signature

Reply via email to