All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Grall <julien@xen.org>
To: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>, Wei Liu <wl@xen.org>,
	Konrad Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ilja Van Sprundel <ivansprundel@ioactive.com>,
	Ian Jackson <ian.jackson@citrix.com>,
	"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Subject: Re: [Xen-devel] [PATCH 4/6] sysctl: use xmalloc_array() for XEN_SYSCTL_page_offline_op
Date: Wed, 5 Feb 2020 17:15:00 +0000	[thread overview]
Message-ID: <6e4a58c4-aa80-f349-67b6-dffb1fa8e707@xen.org> (raw)
In-Reply-To: <40359ffc-13b3-daee-4456-99a461c26de7@suse.com>

Hi Jan,

On 05/02/2020 16:38, Jan Beulich wrote:
> On 05.02.2020 15:34, Julien Grall wrote:
>> On 05/02/2020 13:16, Jan Beulich wrote:
>>> This is more robust than the raw xmalloc_bytes().
>>>
>>> Also add a sanity check on the input page range.
>>
>> It feels to me that the commit message/title should focus on the sanity
>> check. The xmalloc_array() is just a cleanup is "less important".
> 
> But it not being there would generally just result in -ENOMEM
> due to the xmalloc_...() failing (leaving aside overflow not
> accounted for in the old code), which by the new check just
> gets changed into the more applicable -EINVAL. I view the
> changed called out in the title as more important.

None of the commit message really explain this. So the sanity check did 
feel more important.

You probably want to reword the commit message to explain why the sanity 
check is added (i.e ENOMEM vs EINVAL).

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply	other threads:[~2020-02-05 17:15 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-05 13:11 [Xen-devel] [PATCH 0/6] misc hardening and some cleanup Jan Beulich
2020-02-05 13:14 ` [Xen-devel] [PATCH 1/6] EFI: re-check {get, set}-variable name strings after copying in Jan Beulich
2020-02-05 13:14 ` [Xen-devel] [PATCH 2/6] EFI: don't leak heap contents through XEN_EFI_get_next_variable_name Jan Beulich
2020-02-05 13:15 ` [Xen-devel] [PATCH 3/6] xmalloc: guard against integer overflow Jan Beulich
2020-02-05 13:16 ` [Xen-devel] [PATCH 4/6] Arm/GICv2: don't needlessly use xzalloc_bytes() Jan Beulich
2020-02-05 14:29   ` Julien Grall
2020-02-05 13:16 ` [Xen-devel] [PATCH 4/6] sysctl: use xmalloc_array() for XEN_SYSCTL_page_offline_op Jan Beulich
2020-02-05 14:34   ` Julien Grall
2020-02-05 16:38     ` Jan Beulich
2020-02-05 17:15       ` Julien Grall [this message]
2020-02-05 13:17 ` [Xen-devel] [PATCH 6/6] domctl/vNUMA: avoid arithmetic overflow Jan Beulich
2020-02-05 15:13   ` Wei Liu
2020-02-05 13:19 ` [Xen-devel] [PATCH 0/6] misc hardening and some cleanup Jan Beulich

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=6e4a58c4-aa80-f349-67b6-dffb1fa8e707@xen.org \
    --to=julien@xen.org \
    --cc=George.Dunlap@eu.citrix.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ian.jackson@citrix.com \
    --cc=ivansprundel@ioactive.com \
    --cc=jbeulich@suse.com \
    --cc=konrad.wilk@oracle.com \
    --cc=sstabellini@kernel.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /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.