Re: [patch 5/6] IP support for PPP

> > +/****** 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.