Re: [PATCH] rc-core: add separate defines for protocol bitmaps and numbers

[Sean Young <sean@mess.org>]

On Thu, Oct 18, 2012 at 11:59:21PM +0200, David Härdeman wrote:
> On Wed, Oct 17, 2012 at 05:18:56PM +0100, Sean Young wrote:
> >On Fri, Oct 12, 2012 at 01:11:54AM +0200, David Härdeman wrote:
> >> The RC_TYPE_* defines are currently used both where a single protocol is
> >> expected and where a bitmap of protocols is expected. This patch tries
> >> to separate the two in preparation for the following patches.
> >
> >I'm not sure why this is needed.
> 
> I'm not sure I can explain it much better.
> 
> Something like rc_keydown() or functions which add/remove entries to the
> keytable want a single protocol. Future userspace APIs would also
> benefit from numeric protocols (rather than bitmap ones). Keytables are
> smaller if they can use a small(ish) integer rather than a bitmap.
> 
> Other functions or struct members (e.g. allowed_protos,
> enabled_protocols, etc) accept multiple protocols and need a bitmap.
> 
> Using different types reduces the risk of programmer error. Using a
> protocol enum whereever possible also makes for a more future-proof
> user-space API as we don't need to worry about a sufficient number of
> bits being available (e.g. in structs used for ioctl() calls).
> 
> The use of both a number and a corresponding bit is dalso one in e.g.
> the input subsystem as well (see all the references to set/clear bit when
> changing keytables for example).
> 
> >
> >> The intended use is also clearer to anyone reading the code. Where a
> >> single protocol is expected, enum rc_type is used, where one or more
> >> protocol(s) are expected, something like u64 is used.
> >
> >Having two sets of #define and enums for the same information is not
> >necessarily clearer.
> 
> Not only two set of define and enum, two different data types. To me it
> helps a lot to be able to tell from a function declaration whether it
> expects *a* protocol or protocols.

Right, thanks for elaborating. Makes sense.


Sean