On 6/7/19 8:51 AM, David Howells wrote:
> Darrick J. Wong <darrick.w...@oracle.com> wrote:
> 

>>> +   __u32                   info;
>>> +#define WATCH_INFO_OVERRUN 0x00000001      /* Event(s) lost due to overrun 
>>> */
>>> +#define WATCH_INFO_ENOMEM  0x00000002      /* Event(s) lost due to ENOMEM 
>>> */
>>> +#define WATCH_INFO_RECURSIVE       0x00000004      /* Change was recursive 
>>> */
>>> +#define WATCH_INFO_LENGTH  0x000001f8      /* Length of record / 
>>> sizeof(watch_notification) */
>>
>> This is a mask, isn't it?  Could we perhaps have some helpers here?
>> Something along the lines of...
>>
>> #define WATCH_INFO_LENGTH_MASK       0x000001f8
>> #define WATCH_INFO_LENGTH_SHIFT      3
>>
>> static inline size_t watch_notification_length(struct watch_notification *wn)
>> {
>>      return (wn->info & WATCH_INFO_LENGTH_MASK) >> WATCH_INFO_LENGTH_SHIFT *
>>                      sizeof(struct watch_notification);
>> }
>>
>> static inline struct watch_notification *watch_notification_next(
>>              struct watch_notification *wn)
>> {
>>      return wn + ((wn->info & WATCH_INFO_LENGTH_MASK) >>
>>                      WATCH_INFO_LENGTH_SHIFT);
>> }
> 
> No inline functions in UAPI headers, please.  I'd love to kill off the ones
> that we have, but that would break things.

Hi David,

What is the problem with inline functions in UAPI headers?

>> ...so that we don't have to opencode all of the ring buffer walking
>> magic and stuff?
> 
> There'll end up being a small userspace library, I think.

>>> +};
>>> +
>>> +#define WATCH_LENGTH_SHIFT 3
>>> +
>>> +struct watch_queue_buffer {
>>> +   union {
>>> +           /* The first few entries are special, containing the
>>> +            * ring management variables.
>>
>> The first /two/ entries, correct?
> 
> Currently two.
> 
>> Also, weird multiline comment style.
> 
> Not really.

Yes really.

>>> +            */

It does not match the preferred coding style for multi-line comments
according to coding-style.rst.


thanks.
-- 
~Randy

Reply via email to