Re: [PATCH 00/24] ALSA: Generic PCM copy ops using sockptr_t

[Takashi Iwai <tiwai@xxxxxxx>]

On Mon, 31 Jul 2023 21:40:20 +0200,
Mark Brown wrote:
> 
> On Mon, Jul 31, 2023 at 09:30:29PM +0200, Takashi Iwai wrote:
> > Mark Brown wrote:
> 
> > > It really feels like we ought to rename, or add an alias for, the type
> > > if we're going to start using it more widely - it's not helping to make
> > > the code clearer.
> 
> > That was my very first impression, too, but I changed my mind after
> > seeing the already used code.  An alias might work, either typedef or
> > define genptr_t or such as sockptr_t.  But we'll need to copy the
> > bunch of helper functions, too...
> 
> I would predict that if the type becomes more widely used that'll happen
> eventually and the longer it's left the more work it'll be.

That's true.  The question is how more widely it'll be used, then.

Is something like below what you had in mind, too?


Takashi

-- 8< --
From: Takashi Iwai <tiwai@xxxxxxx>
Subject: [PATCH] Introduce uniptr_t as replacement of sockptr_t

Although sockptr_t is used already in several places as a "universal"
pointer, it's still too confusing as if were related with network
stuff.

Make a more generic type, uniptr_t, that does exactly as same as
sockptr_t for a wider use.  sockptr_t becomes now alias to uniptr_t.

Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
 include/linux/sockptr.h | 124 +++++-----------------------------------
 include/linux/uniptr.h  | 117 +++++++++++++++++++++++++++++++++++++
 2 files changed, 132 insertions(+), 109 deletions(-)
 create mode 100644 include/linux/uniptr.h

diff --git a/include/linux/sockptr.h b/include/linux/sockptr.h
index bae5e2369b4f..dc803989a4d6 100644
--- a/include/linux/sockptr.h
+++ b/include/linux/sockptr.h
@@ -1,118 +1,24 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (c) 2020 Christoph Hellwig.
- *
- * Support for "universal" pointers that can point to either kernel or 
userspace
- * memory.
+ * Aliases for the old sockptr_t and its helpers for the new uniptr_t
  */
 #ifndef _LINUX_SOCKPTR_H
 #define _LINUX_SOCKPTR_H
 
-#include <linux/slab.h>
-#include <linux/uaccess.h>
+#include <linux/uniptr.h>
 
-typedef struct {
-       union {
-               void            *kernel;
-               void __user     *user;
-       };
-       bool            is_kernel : 1;
-} sockptr_t;
-
-static inline bool sockptr_is_kernel(sockptr_t sockptr)
-{
-       return sockptr.is_kernel;
-}
-
-static inline sockptr_t KERNEL_SOCKPTR(void *p)
-{
-       return (sockptr_t) { .kernel = p, .is_kernel = true };
-}
-
-static inline sockptr_t USER_SOCKPTR(void __user *p)
-{
-       return (sockptr_t) { .user = p };
-}
-
-static inline bool sockptr_is_null(sockptr_t sockptr)
-{
-       if (sockptr_is_kernel(sockptr))
-               return !sockptr.kernel;
-       return !sockptr.user;
-}
-
-static inline int copy_from_sockptr_offset(void *dst, sockptr_t src,
-               size_t offset, size_t size)
-{
-       if (!sockptr_is_kernel(src))
-               return copy_from_user(dst, src.user + offset, size);
-       memcpy(dst, src.kernel + offset, size);
-       return 0;
-}
-
-static inline int copy_from_sockptr(void *dst, sockptr_t src, size_t size)
-{
-       return copy_from_sockptr_offset(dst, src, 0, size);
-}
-
-static inline int copy_to_sockptr_offset(sockptr_t dst, size_t offset,
-               const void *src, size_t size)
-{
-       if (!sockptr_is_kernel(dst))
-               return copy_to_user(dst.user + offset, src, size);
-       memcpy(dst.kernel + offset, src, size);
-       return 0;
-}
-
-static inline int copy_to_sockptr(sockptr_t dst, const void *src, size_t size)
-{
-       return copy_to_sockptr_offset(dst, 0, src, size);
-}
-
-static inline void *memdup_sockptr(sockptr_t src, size_t len)
-{
-       void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN);
-
-       if (!p)
-               return ERR_PTR(-ENOMEM);
-       if (copy_from_sockptr(p, src, len)) {
-               kfree(p);
-               return ERR_PTR(-EFAULT);
-       }
-       return p;
-}
-
-static inline void *memdup_sockptr_nul(sockptr_t src, size_t len)
-{
-       char *p = kmalloc_track_caller(len + 1, GFP_KERNEL);
-
-       if (!p)
-               return ERR_PTR(-ENOMEM);
-       if (copy_from_sockptr(p, src, len)) {
-               kfree(p);
-               return ERR_PTR(-EFAULT);
-       }
-       p[len] = '\0';
-       return p;
-}
-
-static inline long strncpy_from_sockptr(char *dst, sockptr_t src, size_t count)
-{
-       if (sockptr_is_kernel(src)) {
-               size_t len = min(strnlen(src.kernel, count - 1) + 1, count);
-
-               memcpy(dst, src.kernel, len);
-               return len;
-       }
-       return strncpy_from_user(dst, src.user, count);
-}
-
-static inline int check_zeroed_sockptr(sockptr_t src, size_t offset,
-                                      size_t size)
-{
-       if (!sockptr_is_kernel(src))
-               return check_zeroed_user(src.user + offset, size);
-       return memchr_inv(src.kernel + offset, 0, size) == NULL;
-}
+#define sockptr_t              uniptr_t
+#define sockptr_is_kernel      uniptr_is_kernel
+#define KERNEL_SOCKPTR         KERNEL_UNIPTR
+#define USER_SOCKPTR           USER_UNIPTR
+#define sockptr_is_null                uniptr_is_null
+#define copy_from_sockptr_offset copy_from_uniptr_offset
+#define copy_from_sockptr      copy_from_uniptr
+#define copy_to_sockptr_offset copy_to_uniptr_offset
+#define copy_to_sockptr                copy_to_uniptr
+#define memdup_sockptr         memdup_uniptr
+#define memdup_sockptr_nul     memdup_uniptr_nul
+#define strncpy_from_sockptr   strncpy_from_uniptr
+#define check_zeroed_sockptr   check_zeroed_uniptr
 
 #endif /* _LINUX_SOCKPTR_H */
diff --git a/include/linux/uniptr.h b/include/linux/uniptr.h
new file mode 100644
index 000000000000..3ca9fc8eab4e
--- /dev/null
+++ b/include/linux/uniptr.h
@@ -0,0 +1,117 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2020 Christoph Hellwig.
+ *
+ * Support for "universal" pointers that can point to either kernel or 
userspace
+ * memory.
+ */
+#ifndef _LINUX_UNIPTR_H
+#define _LINUX_UNIPTR_H
+
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+
+typedef struct {
+       union {
+               void            *kernel;
+               void __user     *user;
+       };
+       bool            is_kernel : 1;
+} uniptr_t;
+
+static inline bool uniptr_is_kernel(uniptr_t uniptr)
+{
+       return uniptr.is_kernel;
+}
+
+static inline uniptr_t KERNEL_UNIPTR(void *p)
+{
+       return (uniptr_t) { .kernel = p, .is_kernel = true };
+}
+
+static inline uniptr_t USER_UNIPTR(void __user *p)
+{
+       return (uniptr_t) { .user = p };
+}
+
+static inline bool uniptr_is_null(uniptr_t uniptr)
+{
+       if (uniptr_is_kernel(uniptr))
+               return !uniptr.kernel;
+       return !uniptr.user;
+}
+
+static inline int copy_from_uniptr_offset(void *dst, uniptr_t src,
+                                         size_t offset, size_t size)
+{
+       if (!uniptr_is_kernel(src))
+               return copy_from_user(dst, src.user + offset, size);
+       memcpy(dst, src.kernel + offset, size);
+       return 0;
+}
+
+static inline int copy_from_uniptr(void *dst, uniptr_t src, size_t size)
+{
+       return copy_from_uniptr_offset(dst, src, 0, size);
+}
+
+static inline int copy_to_uniptr_offset(uniptr_t dst, size_t offset,
+                                       const void *src, size_t size)
+{
+       if (!uniptr_is_kernel(dst))
+               return copy_to_user(dst.user + offset, src, size);
+       memcpy(dst.kernel + offset, src, size);
+       return 0;
+}
+
+static inline int copy_to_uniptr(uniptr_t dst, const void *src, size_t size)
+{
+       return copy_to_uniptr_offset(dst, 0, src, size);
+}
+
+static inline void *memdup_uniptr(uniptr_t src, size_t len)
+{
+       void *p = kmalloc_track_caller(len, GFP_USER | __GFP_NOWARN);
+
+       if (!p)
+               return ERR_PTR(-ENOMEM);
+       if (copy_from_uniptr(p, src, len)) {
+               kfree(p);
+               return ERR_PTR(-EFAULT);
+       }
+       return p;
+}
+
+static inline void *memdup_uniptr_nul(uniptr_t src, size_t len)
+{
+       char *p = kmalloc_track_caller(len + 1, GFP_KERNEL);
+
+       if (!p)
+               return ERR_PTR(-ENOMEM);
+       if (copy_from_uniptr(p, src, len)) {
+               kfree(p);
+               return ERR_PTR(-EFAULT);
+       }
+       p[len] = '\0';
+       return p;
+}
+
+static inline long strncpy_from_uniptr(char *dst, uniptr_t src, size_t count)
+{
+       if (uniptr_is_kernel(src)) {
+               size_t len = min(strnlen(src.kernel, count - 1) + 1, count);
+
+               memcpy(dst, src.kernel, len);
+               return len;
+       }
+       return strncpy_from_user(dst, src.user, count);
+}
+
+static inline int check_zeroed_uniptr(uniptr_t src, size_t offset, size_t size)
+{
+       if (!uniptr_is_kernel(src))
+               return check_zeroed_user(src.user + offset, size);
+       return memchr_inv(src.kernel + offset, 0, size) == NULL;
+}
+
+#endif /* _LINUX_UNIPTR_H */
-- 
2.35.3