> For instance, in the wsdisplay_emulops structure, there are: > > int (*alloc_attr)(void *c, int fg, int bg, int flags, long *attrp); > void (*unpack_attr)(void *c, long attr, int *fg, int *bg, int *ul); > > And at the end of the structure is this comment, showing that at > least someone (other than me) was confused by it: > /* XXX need a free_attr() ??? */
`alloc_attr' was named that way because there was a theoretical possibility that drivers would actually need to allocate storage for attribute-related information (which is why attributes are longs rather than uint32_t). A `free_attr' routine would then make sense, except that there is no clear lifetime for attributes (e.g. the kernel messages attribute has to survive free_screen). Since in practice, no driver actually allocates anything and all attributes fit in either 8 bits (vga text mode) or 32 bits (rasops), it would make sense to rename this function to `compute_attr' or `pack_attr' and narrow the attribute type from `long' to `uint32_t'. $.02 Miod
