Linux-Modules Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH RFC 2/3] libkmod: list: export list handling functions
  2017-02-20 14:22       ` Yauheni Kaliuta
@ 2017-02-22  5:26         ` Lucas De Marchi
  0 siblings, 0 replies; 4+ messages in thread
From: Lucas De Marchi @ 2017-02-22  5:26 UTC (permalink / raw)
  To: Yauheni Kaliuta; +Cc: linux-modules, Mian Yousaf Kaukab, bjorn.andersson

On Mon, Feb 20, 2017 at 6:22 AM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> Hi, Lucas!
>
>>>>>> On Mon, 13 Feb 2017 00:05:07 -0800, Lucas De Marchi  wrote:
>
>  > On Fri, Nov 11, 2016 at 3:43 AM, Yauheni Kaliuta
>  > <yauheni.kaliuta@redhat.com> wrote:
>  >> The library uses list functions to create lists internally and
>  >> provides to the clients immutable lists and only functions to
>  >> traverse them.
>  >>
>  >> It may be useful to create own lists in the kmod utilities, so
>  >> export functions for lists creation as well (as it's done for
>  >> arrays). The following functions affected (needed for the
>  >> following depmod modifications):
>  >>
>  >> kmod_list_append()
>  >> kmod_list_remove()
>  >> kmod_list_remove_data()
>  >>
>  >> The patch also adds kmod_list_data() accessor to keep the
>  >> struct kmod_list opaque.
>
>  > I don't see a use for these functions outside of the tools we provide.
>  > We don't need to export them (to external users) in order to use for
>  > our tools.  You can just make the tools include libkmod-internal.h and
>  > use that.
>
>  > In fact since you didn't update the .sym file what will happen is that
>  > external users will see a function in the header but won't be able to
>  > use it since it will not be part of the .so.
>
> Hmm, log macros redefined in this case:
>
>   CC       tools/depmod.o
> In file included from tools/kmod.h:42:0,
>                  from tools/depmod.c:42:
> tools/log.h:31:0: warning: "ERR" redefined
>  #define ERR(...) log_printf(LOG_ERR, __VA_ARGS__)
>
> In file included from tools/depmod.c:40:0:
> ./libkmod/libkmod-internal.h:29:0: note: this is the location of the previous definition
>  #  define ERR(ctx, arg...) kmod_log_cond(ctx, LOG_ERR, ## arg)
>
>
> Would include libkmod-internal.h, then undef then, then include kmod.h be
> acceptable?

This is the approach taken by testsuite. So, yes, it's fine for now.

Since we always include with order system headers, lib headers, local
headers, we could even make the undef on tools/log.h, so inside the
tools we use our local logging macros.

Lucas De Marchi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RFC 2/3] libkmod: list: export list handling functions
  2017-02-13  8:05     ` Lucas De Marchi
@ 2017-02-20 14:22       ` Yauheni Kaliuta
  2017-02-22  5:26         ` Lucas De Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Yauheni Kaliuta @ 2017-02-20 14:22 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: linux-modules, Mian Yousaf Kaukab, bjorn.andersson

Hi, Lucas!

>>>>> On Mon, 13 Feb 2017 00:05:07 -0800, Lucas De Marchi  wrote:

 > On Fri, Nov 11, 2016 at 3:43 AM, Yauheni Kaliuta
 > <yauheni.kaliuta@redhat.com> wrote:
 >> The library uses list functions to create lists internally and
 >> provides to the clients immutable lists and only functions to
 >> traverse them.
 >> 
 >> It may be useful to create own lists in the kmod utilities, so
 >> export functions for lists creation as well (as it's done for
 >> arrays). The following functions affected (needed for the
 >> following depmod modifications):
 >> 
 >> kmod_list_append()
 >> kmod_list_remove()
 >> kmod_list_remove_data()
 >> 
 >> The patch also adds kmod_list_data() accessor to keep the
 >> struct kmod_list opaque.

 > I don't see a use for these functions outside of the tools we provide.
 > We don't need to export them (to external users) in order to use for
 > our tools.  You can just make the tools include libkmod-internal.h and
 > use that.

 > In fact since you didn't update the .sym file what will happen is that
 > external users will see a function in the header but won't be able to
 > use it since it will not be part of the .so.

Hmm, log macros redefined in this case:

  CC       tools/depmod.o
In file included from tools/kmod.h:42:0,
                 from tools/depmod.c:42:
tools/log.h:31:0: warning: "ERR" redefined
 #define ERR(...) log_printf(LOG_ERR, __VA_ARGS__)
 
In file included from tools/depmod.c:40:0:
./libkmod/libkmod-internal.h:29:0: note: this is the location of the previous definition
 #  define ERR(ctx, arg...) kmod_log_cond(ctx, LOG_ERR, ## arg)


Would include libkmod-internal.h, then undef then, then include kmod.h be
acceptable?



-- 
WBR,
Yauheni Kaliuta

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH RFC 2/3] libkmod: list: export list handling functions
  2016-11-11 11:43   ` [PATCH RFC 2/3] libkmod: list: export list handling functions Yauheni Kaliuta
@ 2017-02-13  8:05     ` Lucas De Marchi
  2017-02-20 14:22       ` Yauheni Kaliuta
  0 siblings, 1 reply; 4+ messages in thread
From: Lucas De Marchi @ 2017-02-13  8:05 UTC (permalink / raw)
  To: Yauheni Kaliuta
  Cc: linux-modules, Mian Yousaf Kaukab, bjorn.andersson, Jessica Yu

On Fri, Nov 11, 2016 at 3:43 AM, Yauheni Kaliuta
<yauheni.kaliuta@redhat.com> wrote:
> The library uses list functions to create lists internally and
> provides to the clients immutable lists and only functions to
> traverse them.
>
> It may be useful to create own lists in the kmod utilities, so
> export functions for lists creation as well (as it's done for
> arrays). The following functions affected (needed for the
> following depmod modifications):
>
> kmod_list_append()
> kmod_list_remove()
> kmod_list_remove_data()
>
> The patch also adds kmod_list_data() accessor to keep the
> struct kmod_list opaque.

I don't see a use for these functions outside of the tools we provide.
We don't need to export them (to external users) in order to use for
our tools.  You can just make the tools include libkmod-internal.h and
use that.

In fact since you didn't update the .sym file what will happen is that
external users will see a function in the header but won't be able to
use it since it will not be part of the .so.

Lucas De Marchi

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH RFC 2/3] libkmod: list: export list handling functions
  2016-11-11 11:43 ` [PATCH RFC 0/3] Proposal for cycles handling Yauheni Kaliuta
@ 2016-11-11 11:43   ` Yauheni Kaliuta
  2017-02-13  8:05     ` Lucas De Marchi
  0 siblings, 1 reply; 4+ messages in thread
From: Yauheni Kaliuta @ 2016-11-11 11:43 UTC (permalink / raw)
  To: linux-modules; +Cc: Mian Yousaf Kaukab, bjorn.andersson, Jessica Yu

The library uses list functions to create lists internally and
provides to the clients immutable lists and only functions to
traverse them.

It may be useful to create own lists in the kmod utilities, so
export functions for lists creation as well (as it's done for
arrays). The following functions affected (needed for the
following depmod modifications):

kmod_list_append()
kmod_list_remove()
kmod_list_remove_data()

The patch also adds kmod_list_data() accessor to keep the
struct kmod_list opaque.

Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
---
 libkmod/libkmod-internal.h |  4 ----
 libkmod/libkmod-list.c     | 36 ++++++++++++++++++++++++++++++++++--
 libkmod/libkmod.h          |  5 +++++
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index 4d9db6bc7845..fd1eaed6f556 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -59,11 +59,7 @@ struct kmod_list {
 	void *data;
 };
 
-struct kmod_list *kmod_list_append(struct kmod_list *list, const void *data) _must_check_ __attribute__((nonnull(2)));
 struct kmod_list *kmod_list_prepend(struct kmod_list *list, const void *data) _must_check_ __attribute__((nonnull(2)));
-struct kmod_list *kmod_list_remove(struct kmod_list *list) _must_check_;
-struct kmod_list *kmod_list_remove_data(struct kmod_list *list,
-					const void *data) _must_check_ __attribute__((nonnull(2)));
 struct kmod_list *kmod_list_remove_n_latest(struct kmod_list *list,
 						unsigned int n) _must_check_;
 struct kmod_list *kmod_list_insert_after(struct kmod_list *list, const void *data) __attribute__((nonnull(2)));
diff --git a/libkmod/libkmod-list.c b/libkmod/libkmod-list.c
index 7623693ac12e..8dd445e9829a 100644
--- a/libkmod/libkmod-list.c
+++ b/libkmod/libkmod-list.c
@@ -186,7 +186,15 @@ struct kmod_list *kmod_list_prepend(struct kmod_list *list, const void *data)
 	return new;
 }
 
-struct kmod_list *kmod_list_remove(struct kmod_list *list)
+/**
+ * kmod_list_remove:
+ * @list: the element of the list to remove
+ *
+ * Removes and frees the element @list from the list it is a member of.
+ *
+ * Returns: the new list head value.
+ */
+KMOD_EXPORT struct kmod_list *kmod_list_remove(struct kmod_list *list)
 {
 	struct list_node *node;
 
@@ -202,7 +210,17 @@ struct kmod_list *kmod_list_remove(struct kmod_list *list)
 	return container_of(node, struct kmod_list, node);
 }
 
-struct kmod_list *kmod_list_remove_data(struct kmod_list *list,
+/**
+ * kmod_list_remove_data:
+ * @list: the head of the list
+ * @data: pointer to the data element to remove
+ *
+ * Gets the list head @list, searches for an entry, contaning
+ * data pointer @data and if finds, removes the element from the list.
+ *
+ * Returns: the new list head value.
+ */
+KMOD_EXPORT struct kmod_list *kmod_list_remove_data(struct kmod_list *list,
 							const void *data)
 {
 	struct kmod_list *itr;
@@ -312,3 +330,17 @@ KMOD_EXPORT struct kmod_list *kmod_list_last(const struct kmod_list *list)
 		return NULL;
 	return container_of(list->node.prev, struct kmod_list, node);
 }
+
+/**
+ * kmod_list_data:
+ * @list: list element to extract data pointer from
+ *
+ * Returns the @data pointer from the list entry
+ * to keep the kmod_list structure opaque
+ */
+KMOD_EXPORT void *kmod_list_data(const struct kmod_list *list)
+{
+	if (list == NULL)
+		return NULL;
+	return list->data;
+}
diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
index f9e33c6cde90..a720236dd353 100644
--- a/libkmod/libkmod.h
+++ b/libkmod/libkmod.h
@@ -87,6 +87,11 @@ struct kmod_list *kmod_list_next(const struct kmod_list *list,
 struct kmod_list *kmod_list_prev(const struct kmod_list *list,
 						const struct kmod_list *curr);
 struct kmod_list *kmod_list_last(const struct kmod_list *list);
+struct kmod_list *kmod_list_append(struct kmod_list *list, const void *data) __attribute__((warn_unused_result)) __attribute__((nonnull(2)));
+struct kmod_list *kmod_list_remove(struct kmod_list *list) __attribute__((warn_unused_result));
+struct kmod_list *kmod_list_remove_data(struct kmod_list *list,
+					const void *data) __attribute__((warn_unused_result)) __attribute__((nonnull(2)));
+void *kmod_list_data(const struct kmod_list *list);
 
 #define kmod_list_foreach(list_entry, first_entry) \
 	for (list_entry = first_entry; \
-- 
2.7.4


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-09  9:17 [PATCH v1 2/2] depmod: ignore related modules in depmod_report_cycles Mian Yousaf Kaukab
2016-11-11 11:43 ` [PATCH RFC 0/3] Proposal for cycles handling Yauheni Kaliuta
2016-11-11 11:43   ` [PATCH RFC 2/3] libkmod: list: export list handling functions Yauheni Kaliuta
2017-02-13  8:05     ` Lucas De Marchi
2017-02-20 14:22       ` Yauheni Kaliuta
2017-02-22  5:26         ` Lucas De Marchi

Linux-Modules Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-modules/0 linux-modules/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-modules linux-modules/ https://lore.kernel.org/linux-modules \
		linux-modules@vger.kernel.org linux-modules@archiver.kernel.org
	public-inbox-index linux-modules


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-modules


AGPL code for this site: git clone https://public-inbox.org/ public-inbox