Linux-Modules Archive on lore.kernel.org
 help / color / Atom feed
From: Lucas De Marchi <lucas.de.marchi@gmail.com>
To: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
Cc: linux-modules <linux-modules@vger.kernel.org>,
	Mian Yousaf Kaukab <yousaf.kaukab@suse.com>,
	bjorn.andersson@linaro.org
Subject: Re: [PATCH RFC 2/3] libkmod: list: export list handling functions
Date: Tue, 21 Feb 2017 21:26:46 -0800
Message-ID: <CAKi4VA+4KsvtNje0TVBfp2PtPtyLOBVvYfC5uMJZHBnynWfP=g@mail.gmail.com> (raw)
In-Reply-To: <xuny4lzpueku.fsf@redhat.com>

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

      reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKi4VA+4KsvtNje0TVBfp2PtPtyLOBVvYfC5uMJZHBnynWfP=g@mail.gmail.com' \
    --to=lucas.de.marchi@gmail.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=yauheni.kaliuta@redhat.com \
    --cc=yousaf.kaukab@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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