LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Petr Mladek <pmladek@suse.com>
Cc: Jiri Kosina <jikos@kernel.org>, Miroslav Benes <mbenes@suse.cz>,
	Jason Baron <jbaron@akamai.com>,
	Joe Lawrence <joe.lawrence@redhat.com>,
	Evgenii Shatokhin <eshatokhin@virtuozzo.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v13 02/12] livepatch: Helper macros to define livepatch structures
Date: Wed, 17 Oct 2018 13:17:56 -0500
Message-ID: <20181017181756.cy65ujygmf3ulmiz@treble> (raw)
In-Reply-To: <20181015123713.25868-3-pmladek@suse.com>

On Mon, Oct 15, 2018 at 02:37:03PM +0200, Petr Mladek wrote:
> The definition of struct klp_func might be a bit confusing.
> The original function is defined by name as a string.
> The new function is defined by name as a function pointer
> casted to unsigned long.
> 
> This patch adds helper macros that hide the different types.
> The functions are defined just by the name. For example:

On one hand, these macros are kind of nice, because they do the function
pointer casting for the user.

On the other hand, they hide the field names, which hurts readability a
bit.  For example, it would be easy to accidentally assign the wrong
callback function.

Also, it's unfortunate that these macros are needed in the first place.

What if we just change new_addr (and old_addr) to be 'void *'?  Then the
macros wouldn't be as useful, and we could just get rid of them.

-- 
Josh

  reply index

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-15 12:37 [PATCH v13 00/12] livepatch: Atomic replace feature Petr Mladek
2018-10-15 12:37 ` [PATCH v13 01/12] livepatch: Change void *new_func -> unsigned long new_addr in struct klp_func Petr Mladek
2018-10-15 12:37 ` [PATCH v13 02/12] livepatch: Helper macros to define livepatch structures Petr Mladek
2018-10-17 18:17   ` Josh Poimboeuf [this message]
2018-10-18 11:11     ` Petr Mladek
2018-10-18 12:58       ` Josh Poimboeuf
2018-10-24 11:28         ` Petr Mladek
2018-11-21 12:17           ` Miroslav Benes
2018-10-15 12:37 ` [PATCH v13 03/12] livepatch: Shuffle klp_enable_patch()/klp_disable_patch() code Petr Mladek
2018-10-15 12:37 ` [PATCH v13 04/12] livepatch: Consolidate klp_free functions Petr Mladek
2018-10-17 18:22   ` Josh Poimboeuf
2018-10-18 11:40     ` Petr Mladek
2018-11-21 13:59   ` Miroslav Benes
2018-11-21 14:40     ` Petr Mladek
2018-10-15 12:37 ` [PATCH v13 05/12] livepatch: Refuse to unload only livepatches available during a forced transition Petr Mladek
2018-10-17 18:35   ` Josh Poimboeuf
2018-10-18 12:09     ` Petr Mladek
2018-10-18 13:00       ` Josh Poimboeuf
2018-10-15 12:37 ` [PATCH v13 06/12] livepatch: Simplify API by removing registration step Petr Mladek
2018-10-17 19:06   ` Josh Poimboeuf
2018-10-18 12:33     ` Petr Mladek
2018-10-15 12:37 ` [PATCH v13 07/12] livepatch: Use lists to manage patches, objects and functions Petr Mladek
2018-10-17 20:31   ` Josh Poimboeuf
2018-10-18 12:34     ` Petr Mladek
2018-10-15 12:37 ` [PATCH v13 08/12] livepatch: Add atomic replace Petr Mladek
2018-11-23 12:00   ` Miroslav Benes
2018-10-15 12:37 ` [PATCH v13 09/12] livepatch: Remove Nop structures when unused Petr Mladek
2018-10-17 20:48   ` Josh Poimboeuf
2018-10-18 12:40     ` Petr Mladek
2018-10-15 12:37 ` [PATCH v13 10/12] livepatch: Atomic replace and cumulative patches documentation Petr Mladek
2018-10-15 12:37 ` [PATCH v13 11/12] livepatch: Remove ordering and refuse loading conflicting patches Petr Mladek
2018-10-15 12:37 ` [PATCH v13 12/12] selftests/livepatch: introduce tests Petr Mladek
2018-10-15 19:46   ` Joe Lawrence
2018-10-17 20:48   ` Josh Poimboeuf
2018-10-18 13:34 ` [PATCH v13 00/12] livepatch: Atomic replace feature Josh Poimboeuf

Reply instructions:

You may reply publicly 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=20181017181756.cy65ujygmf3ulmiz@treble \
    --to=jpoimboe@redhat.com \
    --cc=eshatokhin@virtuozzo.com \
    --cc=jbaron@akamai.com \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mbenes@suse.cz \
    --cc=pmladek@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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lore.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lore.kernel.org/lkml/9 lkml/git/9.git

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

Example config snippet for mirrors

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


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