On 6/7/19 8:51 AM, David Howells wrote:

Darrick J. Wong [off-list ref] wrote:

quoted

quoted

+	__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?

quoted

...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.

quoted

quoted

+};
+
+#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.

quoted

Also, weird multiline comment style.

Not really.

Yes really.

quoted

quoted

+		 */

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


thanks.
-- 
~Randy