On 09/03/2012 03:18 AM, Wenchao Xia wrote: > This patch contains internal helper codes. > > Signed-off-by: Wenchao Xia <xiaw...@linux.vnet.ibm.com> > --- > 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 >
> +++ 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 series. > +++ 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. -- Eric Blake ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
