Message ID | 1358357479-7912-2-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On 16 January 2013 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote: > We can provide fast versions based on the other functions defined > by host-utils.h. Some care is required on glibc, which provides > ffsl already. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Hi. This patch breaks compile on MacOS (and likely other BSDs): CC qga/commands-posix.o In file included from qga/commands-posix.c:22: /Users/pm215/src/qemu/include/qemu/host-utils.h:248: error: static declaration of ‘ffsl’ follows non-static declaration (because BSD strings.h provides a system ffsl()). > +/* glibc does not provide an inline version of ffsl, so always define > + * ours. We need to give it a different name, however. > + */ > +#ifdef __GLIBC__ > +#define ffsl qemu_ffsl > +#endif Overriding a system function like this seems a bit brittle -- can't we either (a) always use the system version or (b) always use a QEMU implementation? Failing that, you probably need a configure test to check for system ffsl(). thanks -- PMM
On 30 January 2013 14:05, Peter Maydell <peter.maydell@linaro.org> wrote: > On 16 January 2013 17:31, Paolo Bonzini <pbonzini@redhat.com> wrote: >> We can provide fast versions based on the other functions defined >> by host-utils.h. Some care is required on glibc, which provides >> ffsl already. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > Hi. This patch breaks compile on MacOS (and likely other BSDs): > > CC qga/commands-posix.o > In file included from qga/commands-posix.c:22: > /Users/pm215/src/qemu/include/qemu/host-utils.h:248: error: static > declaration of ‘ffsl’ follows non-static declaration > > (because BSD strings.h provides a system ffsl()). > >> +/* glibc does not provide an inline version of ffsl, so always define >> + * ours. We need to give it a different name, however. >> + */ >> +#ifdef __GLIBC__ >> +#define ffsl qemu_ffsl >> +#endif > > Overriding a system function like this seems a bit brittle -- > can't we either (a) always use the system version or (b) > always use a QEMU implementation? > > Failing that, you probably need a configure test to check for > system ffsl(). Also, we already have a roll-your-own ffsl, it's bitops_ffsl() in bitops.h. Can we improve and use that implementation instead of having a second one? -- PMM
On Wed, Jan 16, 2013 at 06:31:08PM +0100, Paolo Bonzini wrote: > We can provide fast versions based on the other functions defined > by host-utils.h. Some care is required on glibc, which provides > ffsl already. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > include/qemu/host-utils.h | 26 ++++++++++++++++++++++++++ > 1 files changed, 26 insertions(+), 0 deletions(-) Speaking of ffsl() issues, mingw32 now complains about ffsl() being undefined in hbitmap: In file included from /home/stefanha/qemu/include/block/block_int.h:35:0, from qemu-img.c:32: /home/stefanha/qemu/include/qemu/hbitmap.h: In function 'hbitmap_iter_next': /home/stefanha/qemu/include/qemu/hbitmap.h:173:5: warning: implicit declaration of function 'ffsl' [-Wimplicit-function-declaration] Using mingw32-gcc-4.7.2-7.fc18. I haven't looked into this but I thought ffsl() was a gcc built-in and it wouldn't require any headers. Stefan
diff --git a/include/qemu/host-utils.h b/include/qemu/host-utils.h index 81c9a75..2a32be4 100644 --- a/include/qemu/host-utils.h +++ b/include/qemu/host-utils.h @@ -26,6 +26,7 @@ #define HOST_UTILS_H 1 #include "qemu/compiler.h" /* QEMU_GNUC_PREREQ */ +#include <string.h> /* ffsl */ #if defined(__x86_64__) #define __HAVE_FAST_MULU64__ @@ -237,4 +238,29 @@ static inline int ctpop64(uint64_t val) #endif } +/* glibc does not provide an inline version of ffsl, so always define + * ours. We need to give it a different name, however. + */ +#ifdef __GLIBC__ +#define ffsl qemu_ffsl +#endif +static inline int ffsl(long val) +{ + if (!val) { + return 0; + } + +#if QEMU_GNUC_PREREQ(3, 4) + return __builtin_ctzl(val) + 1; +#else + if (sizeof(long) == 4) { + return ctz32(val) + 1; + } else if (sizeof(long) == 8) { + return ctz64(val) + 1; + } else { + abort(); + } +#endif +} + #endif
We can provide fast versions based on the other functions defined by host-utils.h. Some care is required on glibc, which provides ffsl already. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- include/qemu/host-utils.h | 26 ++++++++++++++++++++++++++ 1 files changed, 26 insertions(+), 0 deletions(-)