2012-08-30 13:19:00

by Anderson Lizardo

[permalink] [raw]
Subject: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions

Using this macro in configure.ac enables certain extensions that BlueZ
currently depends on. The macro is recommended instead of defining
_GNU_SOURCE on each C file.
---
configure.ac | 2 ++
src/adapter.c | 1 -
src/event.c | 1 -
src/sdp-xml.c | 1 -
src/textfile.c | 1 -
tools/ciptool.c | 1 -
tools/hciattach.c | 1 -
tools/rfcomm.c | 1 -
8 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index 7d9a34d..6c897d4 100644
--- a/configure.ac
+++ b/configure.ac
@@ -10,6 +10,8 @@ AM_MAINTAINER_MODE

PKG_PROG_PKG_CONFIG

+AC_USE_SYSTEM_EXTENSIONS
+
AC_INIT_BLUEZ

COMPILER_FLAGS
diff --git a/src/adapter.c b/src/adapter.c
index 50779fd..e400603 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -26,7 +26,6 @@
#include <config.h>
#endif

-#define _GNU_SOURCE
#include <stdio.h>
#include <errno.h>
#include <unistd.h>
diff --git a/src/event.c b/src/event.c
index 1586293..3b49dce 100644
--- a/src/event.c
+++ b/src/event.c
@@ -26,7 +26,6 @@
#include <config.h>
#endif

-#define _GNU_SOURCE
#include <stdio.h>
#include <ctype.h>
#include <errno.h>
diff --git a/src/sdp-xml.c b/src/sdp-xml.c
index 52df285..1aca776 100644
--- a/src/sdp-xml.c
+++ b/src/sdp-xml.c
@@ -25,7 +25,6 @@
#include <config.h>
#endif

-#define _GNU_SOURCE
#include <stdio.h>
#include <errno.h>
#include <ctype.h>
diff --git a/src/textfile.c b/src/textfile.c
index 9d88fbc..b6a7e1f 100644
--- a/src/textfile.c
+++ b/src/textfile.c
@@ -25,7 +25,6 @@
#include <config.h>
#endif

-#define _GNU_SOURCE
#include <stdio.h>
#include <errno.h>
#include <ctype.h>
diff --git a/tools/ciptool.c b/tools/ciptool.c
index edce9da..e240f38 100644
--- a/tools/ciptool.c
+++ b/tools/ciptool.c
@@ -25,7 +25,6 @@
#include <config.h>
#endif

-#define _GNU_SOURCE
#include <stdio.h>
#include <errno.h>
#include <stdlib.h>
diff --git a/tools/hciattach.c b/tools/hciattach.c
index d19fa33..e166730 100644
--- a/tools/hciattach.c
+++ b/tools/hciattach.c
@@ -27,7 +27,6 @@
#include <config.h>
#endif

-#define _GNU_SOURCE
#include <stdio.h>
#include <errno.h>
#include <fcntl.h>
diff --git a/tools/rfcomm.c b/tools/rfcomm.c
index e8bdd0f..affdbc7 100644
--- a/tools/rfcomm.c
+++ b/tools/rfcomm.c
@@ -25,7 +25,6 @@
#include <config.h>
#endif

-#define _GNU_SOURCE
#include <stdio.h>
#include <errno.h>
#include <fcntl.h>
--
1.7.9.5



2012-08-31 16:11:39

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions

On Fri, Aug 31, 2012 at 1:02 PM, Marcel Holtmann <[email protected]> wrote:
> Hi Lucas,
>
>> > On Thu, Aug 30, 2012 at 11:08 AM, Marcel Holtmann <[email protected]> wrote:
>> >> Hi Anderson,
>> >>
>> >>> Using this macro in configure.ac enables certain extensions that BlueZ
>> >>> currently depends on. The macro is recommended instead of defining
>> >>> _GNU_SOURCE on each C file.
>> >>
>> >> what is the advantage of this. I am actually fine with using _GNU_SOURCE
>> >> in the C files. It is according to the man pages.
>> >
>> > The only advantage I see is that we don't have to worry about
>> > reviewing these defines as the symbols get incorporated in newer
>> > standards. For instance, this patch was brought up because O_CLOEXEC
>> > does not exist on POSIX.1-2001, but was incorporated POSIX.1-2008, so
>> > for newer systems _GNU_SOURCE is not necessary for it anymore, but for
>> > some (still maintained) distros it is.
>> >
>> > Sometimes we remove code that used these extensions, and simply forget
>> > the _GNU_SOURCE there as well.
>>
>> And also forget to add it. Then 3 months later comes a patch to fix it
>> by adding the definition.
>>
>> Using the autofoo macro we stop the build-fix patches for things like this.
>>
>>
>> That is: it removes code, it's more future-proof and it has no
>> downsides. So, what's the point of not using it?
>
> unless that autoconf thing breaks and we don't get what we want. Or we
> are dealing with an outdated autoconf.

seriously? before 2.60? It's already a pre-requisite in our configure.ac


>
> I am against this. I rather get build fixes and have them recorded in
> git compared to hoping that this magically fixes everything.
>
>> Some months ago I sent a patch to also remove a lot of dumb "#include
>> config.h". It has the same reasoning behind it: remove code and be
>> more future-proof. It was for connman, but if you are interested I can
>> send it to bluez/ofono/etc too -
>> http://permalink.gmane.org/gmane.linux.network.connman/7310
>
> Same thing. I believe that boilerplate is better being explicit than
> magically having it done.

If it helps to convince you: systemd, kmod, pulseaudio, libabc, and
many others.... all of them use these macros.


Lucas De Marchi

2012-08-31 16:02:04

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions

Hi Lucas,

> > On Thu, Aug 30, 2012 at 11:08 AM, Marcel Holtmann <[email protected]> wrote:
> >> Hi Anderson,
> >>
> >>> Using this macro in configure.ac enables certain extensions that BlueZ
> >>> currently depends on. The macro is recommended instead of defining
> >>> _GNU_SOURCE on each C file.
> >>
> >> what is the advantage of this. I am actually fine with using _GNU_SOURCE
> >> in the C files. It is according to the man pages.
> >
> > The only advantage I see is that we don't have to worry about
> > reviewing these defines as the symbols get incorporated in newer
> > standards. For instance, this patch was brought up because O_CLOEXEC
> > does not exist on POSIX.1-2001, but was incorporated POSIX.1-2008, so
> > for newer systems _GNU_SOURCE is not necessary for it anymore, but for
> > some (still maintained) distros it is.
> >
> > Sometimes we remove code that used these extensions, and simply forget
> > the _GNU_SOURCE there as well.
>
> And also forget to add it. Then 3 months later comes a patch to fix it
> by adding the definition.
>
> Using the autofoo macro we stop the build-fix patches for things like this.
>
>
> That is: it removes code, it's more future-proof and it has no
> downsides. So, what's the point of not using it?

unless that autoconf thing breaks and we don't get what we want. Or we
are dealing with an outdated autoconf.

I am against this. I rather get build fixes and have them recorded in
git compared to hoping that this magically fixes everything.

> Some months ago I sent a patch to also remove a lot of dumb "#include
> config.h". It has the same reasoning behind it: remove code and be
> more future-proof. It was for connman, but if you are interested I can
> send it to bluez/ofono/etc too -
> http://permalink.gmane.org/gmane.linux.network.connman/7310

Same thing. I believe that boilerplate is better being explicit than
magically having it done.

Regards

Marcel



2012-08-30 20:02:43

by Lucas De Marchi

[permalink] [raw]
Subject: Re: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions

Hi Marcel,

On Thu, Aug 30, 2012 at 12:26 PM, Anderson Lizardo
<[email protected]> wrote:
> Hi Marcel,
>
> On Thu, Aug 30, 2012 at 11:08 AM, Marcel Holtmann <[email protected]> wrote:
>> Hi Anderson,
>>
>>> Using this macro in configure.ac enables certain extensions that BlueZ
>>> currently depends on. The macro is recommended instead of defining
>>> _GNU_SOURCE on each C file.
>>
>> what is the advantage of this. I am actually fine with using _GNU_SOURCE
>> in the C files. It is according to the man pages.
>
> The only advantage I see is that we don't have to worry about
> reviewing these defines as the symbols get incorporated in newer
> standards. For instance, this patch was brought up because O_CLOEXEC
> does not exist on POSIX.1-2001, but was incorporated POSIX.1-2008, so
> for newer systems _GNU_SOURCE is not necessary for it anymore, but for
> some (still maintained) distros it is.
>
> Sometimes we remove code that used these extensions, and simply forget
> the _GNU_SOURCE there as well.

And also forget to add it. Then 3 months later comes a patch to fix it
by adding the definition.

Using the autofoo macro we stop the build-fix patches for things like this.


That is: it removes code, it's more future-proof and it has no
downsides. So, what's the point of not using it?


Some months ago I sent a patch to also remove a lot of dumb "#include
config.h". It has the same reasoning behind it: remove code and be
more future-proof. It was for connman, but if you are interested I can
send it to bluez/ofono/etc too -
http://permalink.gmane.org/gmane.linux.network.connman/7310


Lucas De Marchi

2012-08-30 15:26:20

by Anderson Lizardo

[permalink] [raw]
Subject: Re: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions

Hi Marcel,

On Thu, Aug 30, 2012 at 11:08 AM, Marcel Holtmann <[email protected]> wrote:
> Hi Anderson,
>
>> Using this macro in configure.ac enables certain extensions that BlueZ
>> currently depends on. The macro is recommended instead of defining
>> _GNU_SOURCE on each C file.
>
> what is the advantage of this. I am actually fine with using _GNU_SOURCE
> in the C files. It is according to the man pages.

The only advantage I see is that we don't have to worry about
reviewing these defines as the symbols get incorporated in newer
standards. For instance, this patch was brought up because O_CLOEXEC
does not exist on POSIX.1-2001, but was incorporated POSIX.1-2008, so
for newer systems _GNU_SOURCE is not necessary for it anymore, but for
some (still maintained) distros it is.

Sometimes we remove code that used these extensions, and simply forget
the _GNU_SOURCE there as well.

It is just a minor build system simplification, if you think it is not
worthy, please reconsider my original patch that adds _GNU_SOURCE just
where it is necessary.

Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

2012-08-30 15:08:53

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH BlueZ] build: Use AC_USE_SYSTEM_EXTENSIONS for POSIX/C extensions

Hi Anderson,

> Using this macro in configure.ac enables certain extensions that BlueZ
> currently depends on. The macro is recommended instead of defining
> _GNU_SOURCE on each C file.

what is the advantage of this. I am actually fine with using _GNU_SOURCE
in the C files. It is according to the man pages.

Regards

Marcel