All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Vernet <void@manifault.com>
To: Petr Mladek <pmladek@suse.com>
Cc: live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, jpoimboe@redhat.com, jikos@kernel.org,
	mbenes@suse.cz, joe.lawrence@redhat.com, corbet@lwn.net
Subject: Re: [PATCH v2] Documentation: livepatch: Add livepatch API page
Date: Thu, 16 Dec 2021 07:00:38 -0800	[thread overview]
Message-ID: <YbtUlkaWSQf4yCIb@dev0025.ash9.facebook.com> (raw)
In-Reply-To: <YbsNcAKzRCxGqXUA@alley>

Petr Mladek <pmladek@suse.com> wrote on Thu [2021-Dec-16 10:57:04 +0100]:

> This change is not good. The function releases all existing shadow
> variables with the given @id for any @obj. And it is not longer clear.

Good point. I'll address that in v3.

> I guess that the primary motivation was to remove  "Inline emphasis
> start-string without end string" mentioned in the commit message.

Yes, this was the primary and only motivation. <*, id> is much clearer and I'm
with you on finding a better alternative.

> A solution would be replace '*' with something else, for example, < , id>.

I think this is better than just obj, but in my opinion this may be confusing
for readers and look like a typo. I think I prefer your second suggestion,
though obj really makes more sense in the case where we're actually passing an
@obj to the function. I'll probably (deservedly?) get lambasted for suggesting
this, but what about taking a page out of rust's book and doing something like
this:

  * klp_shadow_free_all() - detach and free all <_, id> shadow variables
  *		with the given @id.

to indicate that in this case we don't care about the obj. Even for a reader
unfamiliar with rust, hopefully it would get the point across.

> Another solution would be to describe it another way, for example:
> 
>  * klp_shadow_free_all() - detach and free all <obj, id> shadow variables
>  *		with the given @id.

I'm fine with this as well. Let me know what you think about <_, id> vs. what
you suggested, and I'll send out the v3 patch with your preference.

> BTW: There is likely the same problem in Documentation/livepatch/shadow-vars.rst.
>      I see <*, id> there as well.

Indeed you're correct. There's no warning in the build system because there
happen to be two <*, id> ... <*, id> in a row, so rst happily italicizes what's
between them without question. I'll fix this in the v3 of the patch as well.

> Otherwise, the patch looks fine to me.

Thanks for taking a look and for the helpful suggestions.

- David

  reply	other threads:[~2021-12-16 15:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-15 17:47 [PATCH v2] Documentation: livepatch: Add livepatch API page David Vernet
2021-12-16  9:57 ` Petr Mladek
2021-12-16 15:00   ` David Vernet [this message]
2021-12-20 11:24     ` Petr Mladek

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=YbtUlkaWSQf4yCIb@dev0025.ash9.facebook.com \
    --to=void@manifault.com \
    --cc=corbet@lwn.net \
    --cc=jikos@kernel.org \
    --cc=joe.lawrence@redhat.com \
    --cc=jpoimboe@redhat.com \
    --cc=linux-doc@vger.kernel.org \
    --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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.