Re: [Xen-devel] [PATCH V3] xl: create VFB for PV guest when VNC is specified

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Dec 17, 2013 at 11:41:29AM +0000, Ian Campbell wrote:
> On Tue, 2013-12-17 at 11:40 +0000, Wei Liu wrote:
> > On Tue, Dec 17, 2013 at 11:36:29AM +0000, Andrew Cooper wrote:
> > > On 17/12/13 11:28, Wei Liu wrote:
> > > > On Mon, Dec 16, 2013 at 05:46:04PM +0000, Ian Campbell wrote:
> > > > [...]
> > > >>> Changes in V2:
> > > >>> * use macros to reduce code duplication
> > > >>> * vfb=[] take precedence over top level VNC options
> > > >>> ---
> > > >>>  tools/libxl/xl_cmdimpl.c |   89 
> > > >>> ++++++++++++++++++++++++++++++++++++----------
> > > >>>  1 file changed, 70 insertions(+), 19 deletions(-)
> > > >>>
> > > >>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> > > >>> index bd26bcc..6a22e17 100644
> > > >>> --- a/tools/libxl/xl_cmdimpl.c
> > > >>> +++ b/tools/libxl/xl_cmdimpl.c
> > > >>> @@ -315,6 +315,13 @@ static void *xrealloc(void *ptr, size_t sz) {
> > > >>>      return r;
> > > >>>  }
> > > >>>  
> > > >>> +#define ARRAY_EXTEND(array,count)                                    
> > > >>>    \
> > > >>> +    do {                                                             
> > > >>>    \
> > > >>> +        (array) = xrealloc((array),                                  
> > > >>>    \
> > > >>> +                           sizeof(typeof(*(array))) * ((count) + 
> > > >>> 1));   \
> > > >> sizeof(typeof(*array)) == sizeof(*array), doesn't it?
> > > >>
> > > >>> +        (count) = (count) + 1;                                       
> > > >>>    \
> > > >> (count++)
> > > >>
> > > >> I suppose in a macro like this it is tricky to arrange to only evaluate
> > > >> the arguments once, since you need to read and write, but I think you
> > > >> can arrange to evaluate array exactly once instead of 3 times and count
> > > >> just once.
> > > >>
> > > > I came up with this. Is it what you asked for?
> > > >
> > > > #define ARRAY_EXTEND(type_ptr,ptr,type_count,count,ret)                 
> > > > \
> > > >     do {                                                                
> > > > \
> > > >         type_ptr **__ptr = &(ptr);                                      
> > > > \
> > > >         type_count *__count = &(count);                                 
> > > > \
> > > >         *__ptr = xrealloc(*__ptr,                                       
> > > > \
> > > >                           sizeof(**__ptr) * (*__count + 1));            
> > > > \
> > > >         (ret) = *__ptr + *__count;                                      
> > > > \
> > > >         (*__count)++;                                                   
> > > > \
> > > >     } while (0)
> > > >
> > > > Wei.
> > > 
> > > Use typeof() rather than passing the types in directly.  It is a GCC
> > > extension, which tries its best not to evaluate its argument.
> > > 
> > 
> > Oh I thought typeof((count)) always evaluates its argument, good to
> > know. ;-)
> 
> Me too, which is why I didn't suggest it earlier.
> 
> 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?

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel