[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [[UNIKRAFT RFC PATCH] 1/2] lib/isrlib: Applied Simon's recommendations



Hi Christian,

you should add a version number to this patch series and you can keep the title of the series. As comment to the version you can say that you applied my recommendations ;-). Overall, this RFC version looks ways cleaner, thanks a lot for the update! See my comments inline.

Thanks,

Simon

On 10.08.20 19:42, cristian-vijelie wrote:
From: Cristian Vijelie <cristianvijelie@xxxxxxxxx>

---
  include/uk/isr/string.h                       |  53 +++++++
  lib/irslib/Config.uk                          |   7 +
  lib/irslib/Makefile.uk                        |   6 +
  .../include/isrlib-internal/shareddefs.h      | 146 ++++++++++++++++++
  lib/irslib/string.c                           |  11 ++
  5 files changed, 223 insertions(+)
  create mode 100644 include/uk/isr/string.h
  create mode 100644 lib/irslib/Config.uk
  create mode 100644 lib/irslib/Makefile.uk
  create mode 100644 lib/irslib/include/isrlib-internal/shareddefs.h
  create mode 100644 lib/irslib/string.c

diff --git a/include/uk/isr/string.h b/include/uk/isr/string.h
new file mode 100644
index 0000000..ceb8a0a
--- /dev/null
+++ b/include/uk/isr/string.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
+ *
+ *
+ * Copyright (c) 2017, NEC Europe Ltd., NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.

Please remove this line from the headers. It is there by mistake. This needs to be fixed in general.

+ */
+
+#ifndef __UK_ISR_STRING_H__
+#define __UK_ISR_STRING_H__
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define __NEED_size_t
+
+#include <isrlib-internal/shareddefs.h>
Just include the standard libc headers that define you datatypes that you need. I think it is just `size_t` here. You won't need a library-internal shareddefs.h for isrlib. For this header, I would probably include <string.h>.

+
+void *memcpy_isr(void *dst, const void *src, size_t len);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* __STRING_H__ */
diff --git a/lib/irslib/Config.uk b/lib/irslib/Config.uk
new file mode 100644
index 0000000..88ff441
--- /dev/null
+++ b/lib/irslib/Config.uk
@@ -0,0 +1,7 @@
+menuconfig LIBISRLIB
+    bool "isr helper library"
+    default n
+
+if LIBISRLIB
+
+endif
diff --git a/lib/irslib/Makefile.uk b/lib/irslib/Makefile.uk
new file mode 100644
index 0000000..fe2461e
--- /dev/null
+++ b/lib/irslib/Makefile.uk
@@ -0,0 +1,6 @@
+$(eval $(call addlib_s,libisrlib,$(CONFIG_LIBISRLIB)))
+
+CINCLUDES-$(CONFIG_LIBISRLIB)   += -I$(LIBISRLIB_BASE)/include
+CXXINCLUDES-$(CONFIG_LIBISRLIB)   += -I$(LIBISRLIB_BASE)/include
+
+LIBISRLIB_SRCS-y += $(LIBISRLIB_BASE)/string.c|isr
diff --git a/lib/irslib/include/isrlib-internal/shareddefs.h 
b/lib/irslib/include/isrlib-internal/shareddefs.h
new file mode 100644
index 0000000..d386820
--- /dev/null
+++ b/lib/irslib/include/isrlib-internal/shareddefs.h
@@ -0,0 +1,146 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * Authors: Florian Schmidt <florian.schmidt@xxxxxxxxx>
+ *
+ * Copyright (c) 2018, NEC Labs Europe, NEC Corporation. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the copyright holder nor the names of its
+ *    contributors may be used to endorse or promote products derived from
+ *    this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ *
+ * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
+ */
+
+/* This header does by design not have include guards, so that it can be
+ * included from multiple files. The __NEED_x macros instead make sure that
+ * only those definitions are included that are required by that specific
+ * file, and only if they haven't been defined on a previous pass through
+ * this file.
+ */
+
+#include <uk/config.h>
+#include <uk/arch/types.h>
+
+#if (defined __NEED_NULL && !defined __DEFINED_NULL)
+#ifdef __cplusplus
+#define NULL 0L
+#else
+#define NULL ((void *) 0)
+#endif
+#define __DEFINED__NULL
+#endif
+
+#if (defined __NEED_size_t && !defined __DEFINED_size_t)
+typedef __sz size_t;
+#define __DEFINED_size_t
+#endif
+
+#if (defined __NEED_ssize_t && !defined __DEFINED_ssize_t)
+typedef __ssz ssize_t;
+#define __DEFINED_ssize_t
+#endif
+
+#if (defined __NEED_off_t && !defined __DEFINED_off_t)
+typedef __off off_t;
+#define __DEFINED_off_t
+#endif
+
+#if CONFIG_HAVE_TIME
+#include <uk/time_types.h>
+#endif
+
+#if (defined __NEED_mode_t && !defined __DEFINED_mode_t)
+typedef unsigned mode_t;
+#define __DEFINED_mode_t
+#endif
+
+#if defined(__NEED_uid_t) && !defined(__DEFINED_uid_t)
+typedef unsigned uid_t;
+#define __DEFINED_uid_t
+#endif
+
+#if defined(__NEED_gid_t) && !defined(__DEFINED_gid_t)
+typedef unsigned gid_t;
+#define __DEFINED_gid_t
+#endif
+
+#if defined(__NEED_useconds_t) && !defined(__DEFINED_useconds_t)
+typedef unsigned useconds_t;
+#define __DEFINED_useconds_t
+#endif
+
+#if defined(__NEED_pid_t) && !defined(__DEFINED_pid_t)
+typedef int pid_t;
+#define __DEFINED_pid_t
+#endif
+
+#if defined(__NEED_id_t) && !defined(__DEFINED_id_t)
+typedef unsigned id_t;
+#define __DEFINED_id_t
+#endif
+
+#if defined(__NEED_dev_t) && !defined(__DEFINED_dev_t)
+typedef __u64 dev_t;
+#define __DEFINED_dev_t
+#endif
+
+#if defined(__NEED_ino_t) && !defined(__DEFINED_ino_t)
+typedef __u64 ino_t;
+#define __DEFINED_ino_t
+#endif
+
+#if defined(__NEED_nlink_t) && !defined(__DEFINED_nlink_t)
+typedef __u32 nlink_t;
+#define __DEFINED_nlink_t
+#endif
+
+#if defined(__NEED_blkcnt_t) && !defined(__DEFINED_blkcnt_t)
+typedef __s64 blkcnt_t;
+#define __DEFINED_blkcnt_t
+#endif
+
+#if defined(__NEED_blksize_t) && !defined(__DEFINED_blksize_t)
+typedef long blksize_t;
+#define __DEFINED_blksize_t
+#endif
+
+#if defined(__NEED_locale_t) && !defined(__DEFINED_locale_t)
+typedef struct __locale_struct *locale_t;
+#define __DEFINED_locale_t
+#endif
+
+#if defined(__NEED_struct_iovec) && !defined(__DEFINED_struct_iovec)
+struct iovec { void *iov_base; size_t iov_len; };
+#define __DEFINED_struct_iovec
+#endif
+
+#if defined(__NEED_fsblkcnt_t) && !defined(__DEFINED_fsblkcnt_t)
+typedef unsigned long long fsblkcnt_t;
+#define __DEFINED_fsblkcnt_t
+#endif
+
+#if defined(__NEED_fsfilcnt_t) && !defined(__DEFINED_fsfilcnt_t)
+typedef unsigned long long fsfilcnt_t;
+#define __DEFINED_fsfilcnt_t
+#endif
diff --git a/lib/irslib/string.c b/lib/irslib/string.c
new file mode 100644
index 0000000..e14e187
--- /dev/null
+++ b/lib/irslib/string.c
@@ -0,0 +1,11 @@
+#include <uk/isr/string.h>
+
+void *memcpy_isr(void *dst, const void *src, size_t len)
+{
+       size_t p;
+
+       for (p = 0; p < len; ++p)
+               *((__u8 *)(((__uptr)dst) + p)) = *((__u8 *)(((__uptr)src) + p));
+
+       return dst;
+}




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.