From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:58883) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8XeJ-0000Fi-73 for qemu-devel@nongnu.org; Mon, 03 Sep 2012 10:29:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1T8Xe8-0000fb-KL for qemu-devel@nongnu.org; Mon, 03 Sep 2012 10:29:11 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11186) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1T8Xe8-0000fX-BA for qemu-devel@nongnu.org; Mon, 03 Sep 2012 10:29:00 -0400 Message-ID: <5044BE96.7080701@redhat.com> Date: Mon, 03 Sep 2012 08:28:38 -0600 From: Eric Blake MIME-Version: 1.0 References: <1346663926-20188-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1346663926-20188-5-git-send-email-xiawenc@linux.vnet.ibm.com> In-Reply-To: <1346663926-20188-5-git-send-email-xiawenc@linux.vnet.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="------------enigBFA8D009C2669202148AD99B" Subject: Re: [Qemu-devel] [PATCH 4/6] libqblock internal used functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wenchao Xia Cc: kwolf@redhat.com, pbonzini@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, stefanha@gmail.com This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enigBFA8D009C2669202148AD99B Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 09/03/2012 03:18 AM, Wenchao Xia wrote: > This patch contains internal helper codes. >=20 > Signed-off-by: Wenchao Xia > --- > block.c | 2 +- > block.h | 1 + > libqblock/libqblock-helper.c | 92 ++++++++++++++++++++++++++++++++++= ++++++++ > libqblock/libqblock-helper.h | 57 ++++++++++++++++++++++++++ > 4 files changed, 151 insertions(+), 1 deletions(-) > create mode 100644 libqblock/libqblock-helper.c > create mode 100644 libqblock/libqblock-helper.h >=20 > +++ b/libqblock/libqblock-helper.c > @@ -0,0 +1,92 @@ > +#include "libqblock-helper.h" No copyright. Shame. I'll quit pointing it out for the rest of the seri= es. > +++ b/libqblock/libqblock-helper.h > + > +/* this file contains helper function used internally. */ > +#define SECTOR_SIZE (512) Hard-coding this feels wrong, in this day and age of disks with 4096 sectors. Why isn't this a per-image property? > +#define SECTOR_SIZE_MASK (0x01ff) > +#define SECTOR_SIZE_BITS_NUM (9) and these computed from a dynamic per-image sector size? > +#define FUNC_FREE free > +#define FUNC_MALLOC malloc > +#define FUNC_CALLOC calloc Is libqblock-helper.h intended to be installed and included in client applications, or is it internal only? If clients can ever use this header, then you need to avoid namespace violations, by using naming such as QB_SECTOR_SIZE, QB_FUNC_FREE, and so forth. > + > +const char *fmt2str(enum QBlockFormat fmt); > +enum QBlockFormat str2fmt(const char *fmt); Even if this header is not intended for clients, you STILL need to avoid namespace pollution, by either ensuring that your library marks visibility annotations to avoid exporting symbols outside of the public namespace, or by renaming even your internal functions to comply with the namespace. In other words, stomping on the name 'fmt2str' and 'str2fmt' as an external function name is just too likely to clash with an arbitrary client linking against your library. --=20 Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --------------enigBFA8D009C2669202148AD99B Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQEcBAEBCAAGBQJQRL6WAAoJEKeha0olJ0Nq6RUH/RI3Yjjk9/PpNI8U60QmKjbQ jNTPPW81J0EybkPNtPMalO1uNUnYwE9yIvCmcB32K8Sbse8k201JtjICt1e71fLt 06cOem0QHZC3dmu31HD2km5cO2wnwkqCZx8wdrrzwSBmB8pNr8jEpRY4v4wlWs3G M6YcHuNno+QkwSB+fWx7Me8QXjC3biOHV60TavJ7V1QzmWwT27PDK8yasSMtRPSu ppksf89LMnTK7Z/Ln/p/rPIJW0D/az8rQ88YbP1UnAdaJosXWl+lu5ectd6555Cn 92akgQD3gBSdpVvbigb4w/XuLxTEGOUJXFYhS02XFxr1iFzpouRquPSJIcRDt/4= =TGeq -----END PGP SIGNATURE----- --------------enigBFA8D009C2669202148AD99B--