[Qemu-devel] Git head build problem (popcountl vs. system headers)

[Qemu-devel] Git head build problem (popcountl vs. system headers)

[Martin Husemann]

I just tried building git HEAD on NetBSD-current and gcc chokes on
a prototype mismatch for popcountl:

util/hbitmap.c has:

static inline int popcountl(unsigned long l)
{
    return BITS_PER_LONG == 32 ? ctpop32(l) : ctpop64(l);
}

while NetBSD's strings.h uses:

unsigned int        popcountl(unsigned long) __constfunc;


"unsigned" looks better to me for a non negative count, but maybe a
configure test and using the system libs would be preferable?

Martin

Re: [Qemu-devel] Git head build problem (popcountl vs. system headers)

[Richard Henderson]

On 2013-04-25 07:47, Martin Husemann wrote:
> I just tried building git HEAD on NetBSD-current and gcc chokes on
> a prototype mismatch for popcountl:
>
> util/hbitmap.c has:
>
> static inline int popcountl(unsigned long l)
> {
>      return BITS_PER_LONG == 32 ? ctpop32(l) : ctpop64(l);
> }
>
> while NetBSD's strings.h uses:
>
> unsigned int        popcountl(unsigned long) __constfunc;

<strings.h> is the K&R header supplanted by ISO <string.h>.
Is there any good reason that we're including it at all?


r~

Re: [Qemu-devel] Git head build problem (popcountl vs. system headers)

[Laszlo Ersek]

On 04/25/13 12:36, Richard Henderson wrote:
> On 2013-04-25 07:47, Martin Husemann wrote:
>> I just tried building git HEAD on NetBSD-current and gcc chokes on
>> a prototype mismatch for popcountl:
>>
>> util/hbitmap.c has:
>>
>> static inline int popcountl(unsigned long l)
>> {
>>      return BITS_PER_LONG == 32 ? ctpop32(l) : ctpop64(l);
>> }
>>
>> while NetBSD's strings.h uses:
>>
>> unsigned int        popcountl(unsigned long) __constfunc;
> 
> <strings.h> is the K&R header supplanted by ISO <string.h>.
> Is there any good reason that we're including it at all?

- <strings.h> is a portable SUS/POSIX header:

http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/strings.h.html

- "popcountl()" in NetBSD's <strings.h> is either a
standards-nonconformance issue, or the the qemu build system doesn't set
up the right SUS/POSIX environment for compilation on NetBSD. (If that's
possible at all, I don't know.)

Laszlo

Re: [Qemu-devel] Git head build problem (popcountl vs. system headers)

[Richard Henderson]

On 2013-04-25 14:38, Laszlo Ersek wrote:
>> <strings.h> is the K&R header supplanted by ISO <string.h>.
>> Is there any good reason that we're including it at all?
>
> - <strings.h> is a portable SUS/POSIX header:
>
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/strings.h.html

Huh.  Ok, fine, but what do we think we're using out of it that
isn't in <string.h>?


r~

Re: [Qemu-devel] Git head build problem (popcountl vs. system headers)

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]

On 04/25/2013 11:09 AM, Richard Henderson wrote:
> On 2013-04-25 14:38, Laszlo Ersek wrote:
>>> <strings.h> is the K&R header supplanted by ISO <string.h>.
>>> Is there any good reason that we're including it at all?
>>
>> - <strings.h> is a portable SUS/POSIX header:
>>
>> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/strings.h.html
> 
> Huh.  Ok, fine, but what do we think we're using out of it that
> isn't in <string.h>?

strcasecmp().  Just because some (many) systems pollute <string.h> with
strcasecmp() without you having to explicitly include <strings.h> (and
POSIX allows but not requires this:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html#tag_15_02_02
says that the str* prefix is reserved in <string.h>), a
strictly-conforming implementation cannot rely on this extension, and
there are systems where you have to get strcasecmp() from <strings.h>,
just as POSIX requires.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] Git head build problem (popcountl vs. system headers)

[Martin Husemann]

On Thu, Apr 25, 2013 at 06:09:36PM +0100, Richard Henderson wrote:
> On 2013-04-25 14:38, Laszlo Ersek wrote:
> >><strings.h> is the K&R header supplanted by ISO <string.h>.
> >>Is there any good reason that we're including it at all?
> >
> >- <strings.h> is a portable SUS/POSIX header:
> >
> >http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/strings.h.html
> 
> Huh.  Ok, fine, but what do we think we're using out of it that
> isn't in <string.h>?

Ok, I can fix the namespace issue (which is real) easily.
But this still leaves two questions open:

 - why not use an unsigned return value for your homegrown version?
 - would it be preferable to use official/optimized versions if
   available?

Thanks,

Martin

Re: [Qemu-devel] Git head build problem (popcountl vs. system headers)

[Peter Maydell]

On 25 April 2013 19:36, Martin Husemann <martin@duskware.de> wrote:
> But this still leaves two questions open:
>
>  - why not use an unsigned return value for your homegrown version?

We follow gcc's API, which returns int for __builtin_popcountl.

>  - would it be preferable to use official/optimized versions if
>    available?

We will -- assuming a reasonably recent gcc we will end
up compiling this to __builtin_popcount() and friends.

(We could maybe do with a bit more rationalisation in
this area, though.)

thanks
-- PMM

Re: [Qemu-devel] Git head build problem (popcountl vs. system headers)

[Martin Husemann]

On Thu, Apr 25, 2013 at 08:36:55PM +0200, Martin Husemann wrote:
> Ok, I can fix the namespace issue (which is real) easily.

Turns out to be a bit harder: qemu does not define (as far as I can tell)
any restricting macro (_POSIX_C_SOURCE or whatever).

Should it?

Martin

Re: [Qemu-devel] Git head build problem (popcountl vs. system headers)

[Blue Swirl]

On Sat, Apr 27, 2013 at 6:50 PM, Martin Husemann <martin@duskware.de> wrote:
> On Thu, Apr 25, 2013 at 08:36:55PM +0200, Martin Husemann wrote:
>> Ok, I can fix the namespace issue (which is real) easily.
>
> Turns out to be a bit harder: qemu does not define (as far as I can tell)
> any restricting macro (_POSIX_C_SOURCE or whatever).
>
> Should it?

I think the easiest fix is to stop QEMU from reusing a reserved
identifier. So popcountl() in util/hbitmap.c should be renamed to
qemu_popcountl().

>
> Martin
>