From mboxrd@z Thu Jan  1 00:00:00 1970
From: Wei Liu <wei.liu2@citrix.com>
Subject: Re: [PATCH V3] xl: create VFB for PV guest when VNC is
 specified
Date: Tue, 17 Dec 2013 15:16:02 +0000
Message-ID: <20131217151602.GA6093@zion.uk.xensource.com>
References: <1387215576-17455-1-git-send-email-wei.liu2@citrix.com>
	<1387215964.21086.54.camel@kazak.uk.xensource.com>
	<20131217112809.GC26994@zion.uk.xensource.com>
	<52B0373D.1080602@citrix.com>
	<20131217114044.GD26994@zion.uk.xensource.com>
	<1387280489.27441.54.camel@kazak.uk.xensource.com>
	<20131217114948.GE26994@zion.uk.xensource.com>
	<21168.26479.367333.933990@mariner.uk.xensource.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Content-Disposition: inline
In-Reply-To: <21168.26479.367333.933990@mariner.uk.xensource.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Ian Jackson <Ian.Jackson@eu.citrix.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>, Wei Liu <wei.liu2@citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, xen-devel@lists.xen.org
List-Id: xen-devel@lists.xenproject.org

On Tue, Dec 17, 2013 at 03:02:07PM +0000, Ian Jackson wrote:
> Wei Liu writes ("Re: [Xen-devel] [PATCH V3] xl: create VFB for PV guest when VNC is specified"):
> > On Tue, Dec 17, 2013 at 11:41:29AM +0000, Ian Campbell wrote:
> ...
> > > In hindsight its main use is probably in making macros only evaluate
> > > once, so if it itself did the evaluation it wouldn't be very useful!
> > 
> > So now the macro becomes:
> > 
> > #define ARRAY_EXTEND(ptr,count,ret)                                     \
> >     do {                                                                \
> >         typeof((ptr)) *__ptr = &(ptr);                                  \
> >         typeof((count)) *__count = &(count);                            \
> >         *__ptr = xrealloc(*__ptr,                                       \
> >                           sizeof(**__ptr) * (*__count + 1));            \
> >         (ret) = *__ptr + *__count;                                      \
> >         (*__count)++;                                                   \
> >     } while (0)
> > 
> > Thoughts?
> 
> I think all of this complexity to avoid evaluating the array and count
> arguments more than once is unnecessary.  Particularly when they're
> both going to be updated.
> 
> But if you are doing to do that, you need to give your local variables
> different names.  Firstly: names starting __ are reserved for the C
> implementation and shouldn't be used for anything at all.  Secondly,
> prefixing the names with something like "_" isn't sufficient in the
> general case and is the wrong habit.  If you need to define a local
> variable in a macro you should decorate it with something related to
> the name of the macro.  (Hopefully no-one will invoke the same macro
> in one of its own arguments.)
> 
> Please make the macro return the new array entry as Ian C suggested.
> 

OK.

> Finally: IMO the macro should probably initialise the new element
> itself.  So it will be more than just "ARRAY_EXTEND" because it will
> have some knowledge of the kind of array it is, and might need to be
> renamed.  Ian C, what do you think ?
> 

Huh? Elements are initialized by different funcitons. You mean I need to
add extra argument to the macro to pass in the initilization function?

Wei.

> Ian.