diff mbox series

[U-Boot,v1,01/11] linux_compat: add kmemdup()

Message ID 20191011074200.30269-2-takahiro.akashi@linaro.org
State Superseded
Delegated to: Tom Rini
Headers show
Series import x509/pkcs7 parsers from linux | expand

Commit Message

AKASHI Takahiro Oct. 11, 2019, 7:41 a.m. UTC
Adding kmemdup() will help improve portability from linux kernel
code (in my case, lib/crypto/x509_cert_parser.c and pkcs7_parser.c).

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/linux/compat.h |  4 ++--
 lib/linux_compat.c     | 11 +++++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)

Comments

Heinrich Schuchardt Oct. 12, 2019, 11:22 a.m. UTC | #1
On 10/11/19 9:41 AM, AKASHI Takahiro wrote:
> Adding kmemdup() will help improve portability from linux kernel
> code (in my case, lib/crypto/x509_cert_parser.c and pkcs7_parser.c).
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>   include/linux/compat.h |  4 ++--
>   lib/linux_compat.c     | 11 +++++++++++
>   2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index d0f51baab407..d23ef50454ce 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -117,6 +117,8 @@ static inline void kmem_cache_destroy(struct kmem_cache *cachep)
>   	free(cachep);
>   }
>
> +void *kmemdup(const void *src, size_t size, int flags);

kememdup() is already implemented in fs/ubifs/ubifs.c and used both in
drivers/mtd/ and in fs/ubifs/.

Why would you want to use a different signature than both the Linux
kernel and current U-Boot?

> +
>   #define DECLARE_WAITQUEUE(...)	do { } while (0)
>   #define add_wait_queue(...)	do { } while (0)
>   #define remove_wait_queue(...)	do { } while (0)
> @@ -346,8 +348,6 @@ struct writeback_control {
>   	unsigned for_sync:1;		/* sync(2) WB_SYNC_ALL writeback */
>   };
>
> -void *kmemdup(const void *src, size_t len, gfp_t gfp);
> -
>   typedef int irqreturn_t;
>
>   struct timer_list {};
> diff --git a/lib/linux_compat.c b/lib/linux_compat.c
> index 6373b4451eb3..dd1e5b3c2087 100644
> --- a/lib/linux_compat.c
> +++ b/lib/linux_compat.c
> @@ -40,3 +40,14 @@ void *kmem_cache_alloc(struct kmem_cache *obj, int flag)
>   {
>   	return malloc_cache_aligned(obj->sz);
>   }
> +
> +void *kmemdup(const void *src, size_t size, int flags)
> +{

You should not duplicate the implementation in the UBI filesystem.
Instead move the existing implementation to a place that can be linked
in your future work.

Best regards

Heinrich

> +	void *p;
> +
> +	p = kmalloc(size, flags);
> +	if (p)
> +		memcpy(p, src, size);
> +
> +	return p;
> +}
>
AKASHI Takahiro Oct. 17, 2019, 3:04 a.m. UTC | #2
On Sat, Oct 12, 2019 at 01:22:15PM +0200, Heinrich Schuchardt wrote:
> On 10/11/19 9:41 AM, AKASHI Takahiro wrote:
> >Adding kmemdup() will help improve portability from linux kernel
> >code (in my case, lib/crypto/x509_cert_parser.c and pkcs7_parser.c).
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> >---
> >  include/linux/compat.h |  4 ++--
> >  lib/linux_compat.c     | 11 +++++++++++
> >  2 files changed, 13 insertions(+), 2 deletions(-)
> >
> >diff --git a/include/linux/compat.h b/include/linux/compat.h
> >index d0f51baab407..d23ef50454ce 100644
> >--- a/include/linux/compat.h
> >+++ b/include/linux/compat.h
> >@@ -117,6 +117,8 @@ static inline void kmem_cache_destroy(struct kmem_cache *cachep)
> >  	free(cachep);
> >  }
> >
> >+void *kmemdup(const void *src, size_t size, int flags);
> 
> kememdup() is already implemented in fs/ubifs/ubifs.c and used both in
> drivers/mtd/ and in fs/ubifs/.

Good catch, I haven't noticed that.
ubifs.c is a very bad place for that function.
# drivers/mtd/mtd*.c doesn't use it as relevant code is guarded by
  "#ifndef __UBOOT__" though.

> Why would you want to use a different signature than both the Linux
> kernel and current U-Boot?
> 
> >+
> >  #define DECLARE_WAITQUEUE(...)	do { } while (0)
> >  #define add_wait_queue(...)	do { } while (0)
> >  #define remove_wait_queue(...)	do { } while (0)
> >@@ -346,8 +348,6 @@ struct writeback_control {
> >  	unsigned for_sync:1;		/* sync(2) WB_SYNC_ALL writeback */
> >  };
> >
> >-void *kmemdup(const void *src, size_t len, gfp_t gfp);
> >-
> >  typedef int irqreturn_t;
> >
> >  struct timer_list {};
> >diff --git a/lib/linux_compat.c b/lib/linux_compat.c
> >index 6373b4451eb3..dd1e5b3c2087 100644
> >--- a/lib/linux_compat.c
> >+++ b/lib/linux_compat.c
> >@@ -40,3 +40,14 @@ void *kmem_cache_alloc(struct kmem_cache *obj, int flag)
> >  {
> >  	return malloc_cache_aligned(obj->sz);
> >  }
> >+
> >+void *kmemdup(const void *src, size_t size, int flags)
> >+{
> 
> You should not duplicate the implementation in the UBI filesystem.
> Instead move the existing implementation to a place that can be linked
> in your future work.

Okay, I'll remove it from ubifs.c.

Thanks,
-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >+	void *p;
> >+
> >+	p = kmalloc(size, flags);
> >+	if (p)
> >+		memcpy(p, src, size);
> >+
> >+	return p;
> >+}
> >
>
diff mbox series

Patch

diff --git a/include/linux/compat.h b/include/linux/compat.h
index d0f51baab407..d23ef50454ce 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -117,6 +117,8 @@  static inline void kmem_cache_destroy(struct kmem_cache *cachep)
 	free(cachep);
 }
 
+void *kmemdup(const void *src, size_t size, int flags);
+
 #define DECLARE_WAITQUEUE(...)	do { } while (0)
 #define add_wait_queue(...)	do { } while (0)
 #define remove_wait_queue(...)	do { } while (0)
@@ -346,8 +348,6 @@  struct writeback_control {
 	unsigned for_sync:1;		/* sync(2) WB_SYNC_ALL writeback */
 };
 
-void *kmemdup(const void *src, size_t len, gfp_t gfp);
-
 typedef int irqreturn_t;
 
 struct timer_list {};
diff --git a/lib/linux_compat.c b/lib/linux_compat.c
index 6373b4451eb3..dd1e5b3c2087 100644
--- a/lib/linux_compat.c
+++ b/lib/linux_compat.c
@@ -40,3 +40,14 @@  void *kmem_cache_alloc(struct kmem_cache *obj, int flag)
 {
 	return malloc_cache_aligned(obj->sz);
 }
+
+void *kmemdup(const void *src, size_t size, int flags)
+{
+	void *p;
+
+	p = kmalloc(size, flags);
+	if (p)
+		memcpy(p, src, size);
+
+	return p;
+}