[patch 5/6] IP support for PPP

[Kristen Carlson Accardi]

On Sun, 14 Mar 2010 13:22:37 -0700
Marcel Holtmann <marcel at 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.