From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:60157)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aIYex-0000va-8L
	for qemu-devel@nongnu.org; Mon, 11 Jan 2016 04:21:07 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <paolo.bonzini@gmail.com>) id 1aIYew-0001Hu-Br
	for qemu-devel@nongnu.org; Mon, 11 Jan 2016 04:21:07 -0500
Sender: Paolo Bonzini <paolo.bonzini@gmail.com>
References: <1448091903-14460-1-git-send-email-caoj.fnst@cn.fujitsu.com>
	<5693688A.9030201@msgid.tls.msk.ru>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <569373FB.7080505@redhat.com>
Date: Mon, 11 Jan 2016 10:20:59 +0100
MIME-Version: 1.0
In-Reply-To: <5693688A.9030201@msgid.tls.msk.ru>
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] PCI: add param check for api
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Michael Tokarev <mjt@tls.msk.ru>, Cao jin <caoj.fnst@cn.fujitsu.com>, qemu-devel@nongnu.org
Cc: qemu-trivial@nongnu.org, mst@redhat.com



On 11/01/2016 09:32, Michael Tokarev wrote:
>> >  
>> > +    assert(size > 0);
>> > +    assert(offset >= PCI_CONFIG_HEADER_SIZE || !offset);
>> > +
> I'd like to see some ACKs/Reviews for this one, in particular why
> size should be != 0.

In fact it should be >= 2, because two bytes are always written below:

    config = pdev->config + offset;
    config[PCI_CAP_LIST_ID] = cap_id;
    config[PCI_CAP_LIST_NEXT] = pdev->config[PCI_CAPABILITY_LIST];

> Also either move offset assert to the below
> "else" clause or rewrite it to be offset == 0 instead if !offset :)

Good idea to move it below, or even to add

    assert(offset >= PCI_CONFIG_HEADER_SIZE);

after the "if", before the "config" assignment.

Paolo