On 6/7/19 8:51 AM, David Howells wrote:
> Darrick J. Wong <darrick.wong@xxxxxxxxxx> 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
