Stefan Hajnoczi <stefa...@gmail.com> writes:
> On Thu, Jul 31, 2014 at 05:14:12PM -0400, John Snow wrote:
>>
>> On 07/31/2014 06:13 AM, Stefan Hajnoczi wrote:
>> >On Wed, Jul 30, 2014 at 06:28:28PM -0400, John Snow wrote:
>> >>-static uint64_t pc_alloc(QGuestAllocator *allocator, size_t size)
>> >>+static inline void mlist_insert(MemList *head, MemBlock *insr)
>> >> {
>> >>- PCAlloc *s = container_of(allocator, PCAlloc, alloc);
>> >>- uint64_t addr;
>> >>+ QTAILQ_INSERT_HEAD(head, insr, MLIST_ENTNAME);
>> >>+}
>> >>+
>> >>+static inline void mlist_append(MemList *head, MemBlock *node)
>> >>+{
>> >>+ QTAILQ_INSERT_TAIL(head, node, MLIST_ENTNAME);
>> >>+}
>> >>+
>> >>+static inline void mlist_unlink(MemList *head, MemBlock *rem)
>> >>+{
>> >>+ QTAILQ_REMOVE(head, rem, MLIST_ENTNAME);
>> >>+}
>> >For the same reasons as my comments about the macros, these trivial
>> >functions
>> >are boilerplate. Why not use the QTAILQ macros directly?
>> For /at least/ the append and insert cases, I desired call-by-value
>> semantics.
>> As a matter of taste, I find macros annoying for the reason that you cannot
>> inline things such as:
>> mlist_insert(list, mlist_new(...));
>>
>> but unlink is certainly superfluous, and just something I did for some
>> consistency.
>>
>> If there is a matter of style where in-line function call is to be avoided
>> entirely, I'll just nix all of these trivial inlines.
>
> As a reviewer I prefer to see familiar APIs rather than a convenience
> layer because it's extra stuff I need to keep in mind. Sticking to the
> APIs makes it quicker for other QEMU developers to parse the code.
Seconded.
> That said, feel free to keep the functions if you want.
Can't say, as I haven't studied the patch in detail.
