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
next prev parent 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.