Re: [patch 5/6] IP support for PPP
On Sun, 14 Mar 2010 13:22:37 -0700
Marcel Holtmann <marcel(a)holtmann.org> wrote:
> +/****** IPCP support ****************/
> +enum {
> + /* supported codes */
> + IPCP_SUPPORTED_CODES = (1 << CONFIGURE_REQUEST) |
> + (1 << CONFIGURE_ACK) |
> + (1 << CONFIGURE_NAK) |
> + (1 << CONFIGURE_REJECT) |
> + (1 << TERMINATE_REQUEST) |
> + (1 << TERMINATE_ACK) |
> + (1 << CODE_REJECT),
> +
> + IPCP_PROTO = 0x8021,
> +
> + /* option types */
> + IP_ADDRESSES = 1,
> + IP_COMPRESSION_PROTO = 2,
> + IP_ADDRESS = 3,
> + PRIMARY_DNS_SERVER = 129,
> + SECONDARY_DNS_SERVER = 131,
> +};
I don't think enum is the right way for this. I can see it for the
option types, but for IPCP_PROTO and IPCP_SUPPORTED_CODES. I would say
just using simple defines is way better.
Feel free to convince me other way or show me what I have missed here.
It's mostly just a habit, but in general there are advantages to
using enum vs. a define. You are assured that the scope is kept local,
even if you are uncreative with your name choices, whereas with a macro
it is not. Also it's sometimes easier to debug with an enum vs. a macro.
I can certainly change it if you really want me too.