[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH V3] xl: create VFB for PV guest when VNC is specified
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. 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 ? Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |