Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 27 Aug 2015 16:15:58 -0500
From:      Pedro Giffuni <pfg@FreeBSD.org>
To:        Alexander Kabaev <kabaev@gmail.com>
Cc:        "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r287206 - head/sys/sys
Message-ID:  <55DF7E0E.6030007@FreeBSD.org>
In-Reply-To: <CAALb-ZXvc5dFB1Hfxmfs9PdcKZZ7=fQ9DZptk2Qz_3Hw%2BwHH_w@mail.gmail.com>
References:  <201508271400.t7RE0Nbc071389@repo.freebsd.org> <55DF3519.1000106@FreeBSD.org> <CAALb-ZXvc5dFB1Hfxmfs9PdcKZZ7=fQ9DZptk2Qz_3Hw%2BwHH_w@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help


On 08/27/15 12:41, Alexander Kabaev wrote:
...

> The existing conditional is not working for lint and I had to shuffle it
> under bigger
> '#ifdef lint' protection.

And other BSDs are just getting rid of lint. I have no opinion on that
though.

>  ... Nor is the condition even well formed enough
> to work with any
> recent GCC and using __has_attribute in naked form is a mistake:
>
> % cat t.c
> #if __has_attribute(alloc_size)
> # error Has attribute
> #endif
>

We are not really using __has_attribute() in it's naked form: we are
following the official clang procedure:

http://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros

In cdefs.h that corresponds to lines 42-44.

...

> % mips-portbld-freebsd11.0-gcc --version
> mips-portbld-freebsd11.0-gcc (FreeBSD Ports Collection for mips) 5.2.0
> Copyright (C) 2015 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> % mips-portbld-freebsd11.0-gcc -E t.c
> # 1 "t.c"
> # 1 "<built-in>"
> # 1 "<command-line>"
> # 1 "t.c"
> t.c:2:3: error: #error Has attribute
>   # error Has attribute
>     ^
>
> % gcc48 --version
> gcc48 (FreeBSD Ports Collection) 4.8.5
> Copyright (C) 2015 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> % gcc48 -E t.c
> # 1 "t.c"
> # 1 "<built-in>"
> # 1 "<command-line>"
> # 1 "t.c"
> t.c:1:20: error: missing binary operator before token "("
>   #if __has_attribute(alloc_size)
>
>         Modified:
>             head/sys/sys/cdefs.h
>
>     ..
>
>
>            #if !__GNUC_PREREQ__(2, 95)
>         @@ -371,24 +382,12 @@
>            #define       __returns_twice
>            #endif
>
>         -#if __has_attribute(alloc_size) || __GNUC_PREREQ__(4, 3)
>         -#define        __alloc_size(x) __attribute__((__alloc_size__(x)))
>         -#else
>         -#define        __alloc_size(x)
>         -#endif
>         -
>
>
>     This surely got through in GCC's case through the __GNUC_PREREQ__.
>     Of course gcc 4.2 has neither attribute but clang has alloc_size
>     so I wonder why it hasn't affected the lint builds.
>
>     Just curiosity, the change is OK but it will be getting ugly if we
>     have to add all the new attributes in the !lint section.
>
>     Regards,
>
>     Pedro.
>
>
> clang in tree does not have the slightest idea about the alloc_size
> attribute, and that is why it worked, as demonstrated by the test below:
>

Ugh yes, it appears it's unsupported still. I though I had seen a bug 
report with a patch but I can't find it anymore. Grepping it in contrib
doesn't show anything.

...

> The original change to cdefs.h appears to be plain wrong.
>

Not really, it may appear some day.

Pedro.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?55DF7E0E.6030007>