[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] tools/toollog: Drop XTL_NEW_LOGGER()
On Tue, 2016-01-19 at 16:40 +0000, Andrew Cooper wrote: > On 19/01/16 16:24, Ian Campbell wrote: > > On Thu, 2016-01-14 at 20:13 +0000, Andrew Cooper wrote: > > > XTL_NEW_LOGGER() makes a number of unreasonable assumptions about the > > > symbols > > > visible in its scope, > > It assumes that the function names to fill in the vtable and the type > > name > > are related, that hardly seems totally "unreasonable". What else does > > it > > assume that makes it unreasonable? > > We have already had this argument once, the result being "patch > welcome".ÂÂHere one is. > > Not only does it assume certain names, it is tokenised with a magic 3rd > identifier. > > It also assumes the presence of a structure member named vtable. The underlying issue with all of these is the _undocumented_ nature of the assumptions, which is certainly a bug, however those assumptions are not in themselves "unreasonable" as was claimed. > > I think if you intend to remove something on this basis you need to be > > specific about what you believe the short comings are. > > GNUism in header file claiming C99 strictness > > If vtable isn't the first element in the structure, it follows a wild > pointer on error. Thank you. Both of these and the lack of documentation should have been spelled out in the original commit message as reasons for the removal. > > > Âand as such is only usable by its sole caller. > > "not usable by every imaginable caller" is not the same as "usable by > > one > > single possible caller", I think you are overstating the case here. > > It is genuinely harder to reverse engineer how to use that macro than to > opencode a sane alternative. A consequence of the lack of documentation rather than any inherent unreasonableness of the provision of such a helper. BTW your patch removes the logging on failure to allocate, which should either be fixed or called out in the commit message. > I stand firmly by my statement. You might reasonably stand by your actual reasons for removing this macro, but the original commit message didn't contain them. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |