From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [RFC][PATCH 11/45] msi: Factor out delivery hook
Date: Tue, 18 Oct 2011 14:49:44 +0200
Message-ID: <20111018124943.GP28776@redhat.com>
References: <cover.1318843693.git.jan.kiszka@siemens.com>
	<d13aab98dc5c08fa2473511e006365b3a4bbee3f.1318843693.git.jan.kiszka@siemens.com>
	<20111017134349.GD6406@redhat.com> <4E9C7EE3.9050603@web.de>
	<20111018120549.GH28776@redhat.com> <4E9D6FC1.9040504@siemens.com>
	<20111018123839.GM28776@redhat.com>
	<alpine.LNX.2.00.1110181644150.7431@linmac>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	Jan Kiszka <jan.kiszka@siemens.com>, Marcelo Tosatti <mtosatti@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Alex Williamson <alex.williamson@redhat.com>, Avi Kivity <avi@redhat.com>
To: malc <av1474@comtv.ru>
Return-path: <qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org>
Content-Disposition: inline
In-Reply-To: <alpine.LNX.2.00.1110181644150.7431@linmac>
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>
Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org
List-Id: kvm.vger.kernel.org

On Tue, Oct 18, 2011 at 04:44:47PM +0400, malc wrote:
> On Tue, 18 Oct 2011, Michael S. Tsirkin wrote:
> 
> > On Tue, Oct 18, 2011 at 02:23:29PM +0200, Jan Kiszka wrote:
> > > On 2011-10-18 14:05, Michael S. Tsirkin wrote:
> > > > On Mon, Oct 17, 2011 at 09:15:47PM +0200, Jan Kiszka wrote:
> > > >> On 2011-10-17 15:43, Michael S. Tsirkin wrote:
> > > >>> On Mon, Oct 17, 2011 at 11:27:45AM +0200, Jan Kiszka wrote:
> > > >>>> diff --git a/hw/msi.c b/hw/msi.c
> > > >>>> index 3c7ebc3..9055155 100644
> > > >>>> --- a/hw/msi.c
> > > >>>> +++ b/hw/msi.c
> > > >>>> @@ -40,6 +40,14 @@
> > > >>>>  /* Flag for interrupt controller to declare MSI/MSI-X support */
> > > >>>>  bool msi_supported;
> > > >>>>  
> > > >>>> +static void msi_unsupported(MSIMessage *msg)
> > > >>>> +{
> > > >>>> +    /* If we get here, the board failed to register a delivery handler. */
> > > >>>> +    abort();
> > > >>>> +}
> > > >>>> +
> > > >>>> +void (*msi_deliver)(MSIMessage *msg) = msi_unsupported;
> > > >>>> +
> > > >>>
> > > >>> How about we set this to NULL, and check it instead of the bool
> > > >>> flag?
> > > >>>
> > > >>
> > > >> Yeah. I will introduce
> > > >>
> > > >> bool msi_supported(void)
> > > >> {
> > > >>     return msi_deliver != msi_unsupported;
> > > >> }
> > > >>
> > > >> OK?
> > > >>
> > > >> Jan
> > > >>
> > > > 
> > > > Looks a bit weird ...
> > > > NULL is a pretty standard value for an invalid pointer, isn't it?
> > > 
> > > Save us the runtime check and is equally expressive and readable IMHO.
> > > 
> > > Jan
> > 
> > Do we need to check?
> > NULL dereference leads to a crash just as surely...
> > 
> 
> Not universally (not on AIX for instance (read)).

This is a NULL function call though :)
Anyway, this was just nitpicking. Do it any way you like.

> -- 
> mailto:av1474@comtv.ru