This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH] fix misc issues related to allowingsupport of mo

To: Jan Beulich <jbeulich@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] fix misc issues related to allowingsupport of more CPUs
From: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Date: Mon, 22 Sep 2008 15:02:39 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Mon, 22 Sep 2008 07:03:06 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <C4FD1759.2738D%keir.fraser@xxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AckcjiZIZLxHkIiBEd2psAAX8io7RQALbkrY
Thread-topic: [Xen-devel] [PATCH] fix misc issues related to allowingsupport of more CPUs
User-agent: Microsoft-Entourage/
On 22/9/08 09:35, "Keir Fraser" <keir.fraser@xxxxxxxxxxxxx> wrote:

> On 22/9/08 08:46, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:
>>> Why must it be WARN_ON()? You think you could specify strings so long that
>>> they overflow 32 bits? You've got other problems in that case.
>> No, that's not the reason. It's because of how bitmap_scnprintf() and
>> bitmap_scnlistprintf() work - they can (validly, assuming that the code
>> having been derived from Linux and still being that way in Linux, hence
>> apparently considered correct) pass negative sizes to scnprintf(), and
>> hence it must not kill the system to actually do so.
> The obvious answer would be to fix the bogus callers. Or consider negative
> size to be a valid input. Warning on what callers consider valid behaviour
> is just weird. I would say the former (fix the callers) is the better way to
> go here; presumably they just need to clamp the size parameter.

Further to this, actually I think the callers in bitmap.c are correct.
Assuming len<=buflen at the start of the bitmap_scn*() functions (valid
since buflen is non-negative and len == 0 at function start) then we'll
never have len>buflen because scnprintf() truncates its return value to be
less than its size parameter, which is always buflen-len.

What *is* a bug is that scnprintf() returns -1 when passed a size==0, which
it specifically is not supposed to do! I notice this bug appears to live on
in current Linux 2.6 too.

I will fix this bug and extend the vsnprintf() BUG_ON() with the predicate
you used for your WARN_ON() logic. Beyond that I don't think anything needs
to be done.

 -- Keir

Xen-devel mailing list