From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59279) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XDERj-0004WD-SW for qemu-devel@nongnu.org; Fri, 01 Aug 2014 11:08:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XDERa-00044F-R4 for qemu-devel@nongnu.org; Fri, 01 Aug 2014 11:08:39 -0400 Received: from mail-we0-x22c.google.com ([2a00:1450:400c:c03::22c]:33741) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XDERa-00043p-Jy for qemu-devel@nongnu.org; Fri, 01 Aug 2014 11:08:30 -0400 Received: by mail-we0-f172.google.com with SMTP id x48so4460227wes.17 for ; Fri, 01 Aug 2014 08:08:29 -0700 (PDT) Date: Fri, 1 Aug 2014 16:08:25 +0100 From: Stefan Hajnoczi Message-ID: <20140801150825.GE11091@stefanha-thinkpad.redhat.com> References: <1406759309-13742-1-git-send-email-jsnow@redhat.com> <1406759309-13742-4-git-send-email-jsnow@redhat.com> <20140731101351.GD25929@stefanha-thinkpad.redhat.com> <53DAB1A4.7030900@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Ns7jmDPpOpCD+GE/" Content-Disposition: inline In-Reply-To: <53DAB1A4.7030900@redhat.com> Subject: Re: [Qemu-devel] [PATCH 3/4] libqos: add a simple first-fit memory allocator List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow Cc: marc.mari.barcelo@gmail.com, pbonzini@redhat.com, qemu-devel@nongnu.org, Stefan Hajnoczi --Ns7jmDPpOpCD+GE/ Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jul 31, 2014 at 05:14:12PM -0400, John Snow wrote: >=20 > 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 =3D 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 func= tions > >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 cann= ot > inline things such as: > mlist_insert(list, mlist_new(...)); >=20 > but unlink is certainly superfluous, and just something I did for some > consistency. >=20 > 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. That said, feel free to keep the functions if you want. Stefan --Ns7jmDPpOpCD+GE/ Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJT261pAAoJEJykq7OBq3PItmIIAJfSedVV5x65TYz4DX8jKn+r Ch4pWWsvSt3mSXWzrnj0O0KgQ68kMevyIiDBxLsFzPuiLcwcwQL5Q8ZM0sf9lAnx QdygPOIHzweY6rnJx+qaPwuYn2v75V1AnTlNx2m0oT+cyniQbmHM818GSwfptuHB BZu3BQUfeaYLkF8BPixgSl9W3e1PJ9Y/CgfdDlcG3OUZyMkzBZGUxChXTj4XuA3K tTicToUljINbTrq3irKlPGPmjkvanooYILCzW+u9NKddoh89KzioID1OzrCVaqcO eFS3UlGXD7ubS+Uf3fdvhkwYas/33jKYjsf0nz7AtQ0tUlpN48RkF1cJfsOwnTA= =YkdK -----END PGP SIGNATURE----- --Ns7jmDPpOpCD+GE/--