[PATCH 1/3] bnep: avoid use of caddr_t

[PATCH 1/3] bnep: avoid use of caddr_t

[Natanael Copa]

caddr_t is legacy BSD and should be avoided.

This fixes building against musl libc.
---
 profiles/network/bnep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 2a74016..4f9b801 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -202,7 +202,7 @@ static int bnep_if_up(const char *devname)
 	ifr.ifr_flags |= IFF_UP;
 	ifr.ifr_flags |= IFF_MULTICAST;
 
-	err = ioctl(sk, SIOCSIFFLAGS, (caddr_t) &ifr);
+	err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
 
 	close(sk);
 
@@ -227,7 +227,7 @@ static int bnep_if_down(const char *devname)
 	ifr.ifr_flags &= ~IFF_UP;
 
 	/* Bring down the interface */
-	err = ioctl(sk, SIOCSIFFLAGS, (caddr_t) &ifr);
+	err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
 
 	close(sk);
 
-- 
1.8.5.3

[PATCH 2/3] various header include fixes for building with musl libc

[Natanael Copa]

we need:
 sys/stat.h for mode_t
 limits.h for PATH_MAX
---
 src/textfile.h  | 2 ++
 tools/csr_usb.c | 1 +
 tools/hid2hci.c | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/textfile.h b/src/textfile.h
index b779bd2..e26da5d 100644
--- a/src/textfile.h
+++ b/src/textfile.h
@@ -24,6 +24,8 @@
 #ifndef __TEXTFILE_H
 #define __TEXTFILE_H
 
+#include <sys/stat.h>
+
 int create_file(const char *filename, const mode_t mode);
 int create_name(char *buf, size_t size, const char *path,
 				const char *address, const char *name);
diff --git a/tools/csr_usb.c b/tools/csr_usb.c
index a483bc1..5fb6bdc 100644
--- a/tools/csr_usb.c
+++ b/tools/csr_usb.c
@@ -33,6 +33,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <dirent.h>
+#include <limits.h>
 #include <sys/ioctl.h>
 
 #include "csr.h"
diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index 95b4abf..2dbfca7 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -35,6 +35,7 @@
 #include <string.h>
 #include <dirent.h>
 #include <getopt.h>
+#include <limits.h>
 #include <sys/ioctl.h>
 #include <linux/types.h>
 #include <linux/hiddev.h>
-- 
1.8.5.3

[PATCH 3/3] unit: prevent use of glibc's error(3)

[Natanael Copa]

When building the test-sdp we don't want src/sdpd-request.c end up
using the incompatible GNU libc's error(3).

This also fixes building on musl libc which misses the error(3) GNU
extension.
---
 unit/test-sdp.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index 6d699e2..eeed0cb 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -134,6 +134,12 @@ void btd_debug(const char *format, ...)
 {
 }
 
+void error(const char *format, ...);
+
+void error(const char *format, ...)
+{
+}
+
 static void context_quit(struct context *context)
 {
 	g_main_loop_quit(context->main_loop);
-- 
1.8.5.3

Re: [PATCH 3/3] unit: prevent use of glibc's error(3)

[Luiz Augusto von Dentz]

Hi Natanael,

On Fri, Jan 17, 2014 at 2:08 PM, Natanael Copa <natanael.copa@gmail.com> wrote:
> When building the test-sdp we don't want src/sdpd-request.c end up
> using the incompatible GNU libc's error(3).
>
> This also fixes building on musl libc which misses the error(3) GNU
> extension.
> ---
>  unit/test-sdp.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/unit/test-sdp.c b/unit/test-sdp.c
> index 6d699e2..eeed0cb 100644
> --- a/unit/test-sdp.c
> +++ b/unit/test-sdp.c
> @@ -134,6 +134,12 @@ void btd_debug(const char *format, ...)
>  {
>  }
>
> +void error(const char *format, ...);
> +
> +void error(const char *format, ...)
> +{
> +}
> +
>  static void context_quit(struct context *context)
>  {
>         g_main_loop_quit(context->main_loop);
> --
> 1.8.5.3

We could perhaps do the same thing we did in test-avdtp.c, build with
log support and add the following check:

if (g_test_verbose())
__btd_log_init("*", 0);


-- 
Luiz Augusto von Dentz

Re: [PATCH 3/3] unit: prevent use of glibc's error(3)

[Natanael Copa]

On Fri, 17 Jan 2014 16:37:15 +0200
Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:

> Hi Natanael,
> 
> On Fri, Jan 17, 2014 at 2:08 PM, Natanael Copa <natanael.copa@gmail.com> wrote:
> > When building the test-sdp we don't want src/sdpd-request.c end up
> > using the incompatible GNU libc's error(3).
> >
> > This also fixes building on musl libc which misses the error(3) GNU
> > extension.
> > ---
> >  unit/test-sdp.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/unit/test-sdp.c b/unit/test-sdp.c
> > index 6d699e2..eeed0cb 100644
> > --- a/unit/test-sdp.c
> > +++ b/unit/test-sdp.c
> > @@ -134,6 +134,12 @@ void btd_debug(const char *format, ...)
> >  {
> >  }
> >
> > +void error(const char *format, ...);
> > +
> > +void error(const char *format, ...)
> > +{
> > +}
> > +
> >  static void context_quit(struct context *context)
> >  {
> >         g_main_loop_quit(context->main_loop);
> > --
> > 1.8.5.3
> 
> We could perhaps do the same thing we did in test-avdtp.c, build with
> log support and add the following check:
> 
> if (g_test_verbose())
> __btd_log_init("*", 0);
> 
> 

sounds good to me.

-nc

Re: [PATCH 1/3] bnep: avoid use of caddr_t

[Luiz Augusto von Dentz]

Hi Natanael,

On Fri, Jan 17, 2014 at 2:08 PM, Natanael Copa <natanael.copa@gmail.com> wrote:
> caddr_t is legacy BSD and should be avoided.
>
> This fixes building against musl libc.

The patches looks good, but could please also add to the description
the actual errors you got.

[PATCH v2 0/3] Various fixes for building bluez with musl libc

[Natanael Copa]

Changes since v1:
- Add the compile errors in commit message
- Build unit testing with log support

Natanael Copa (3):
  various header include fixes for building with musl libc
  bnep: avoid use of caddr_t
  unit: prevent use of glibc's error(3)

 Makefile.am             | 1 +
 profiles/network/bnep.c | 4 ++--
 src/textfile.h          | 2 ++
 tools/csr_usb.c         | 1 +
 tools/hid2hci.c         | 1 +
 unit/test-sdp.c         | 9 +++------
 6 files changed, 10 insertions(+), 8 deletions(-)

-- 
1.8.5.3

[PATCH v2 1/3] various header include fixes for building with musl libc

[Natanael Copa]

we need:
 sys/stat.h for mode_t
 limits.h for PATH_MAX

Fixes compile errors:
In file included from tools/hciconfig.c:45:0:
./src/textfile.h:27:1: error: unknown type name 'mode_t'
 int create_file(const char *filename, const mode_t mode);
 ^

tools/csr_usb.c: In function 'read_value':
tools/csr_usb.c:71:12: error: 'PATH_MAX' undeclared (first use in this function)
  char path[PATH_MAX];
            ^
---
 src/textfile.h  | 2 ++
 tools/csr_usb.c | 1 +
 tools/hid2hci.c | 1 +
 3 files changed, 4 insertions(+)

diff --git a/src/textfile.h b/src/textfile.h
index b779bd2..e26da5d 100644
--- a/src/textfile.h
+++ b/src/textfile.h
@@ -24,6 +24,8 @@
 #ifndef __TEXTFILE_H
 #define __TEXTFILE_H
 
+#include <sys/stat.h>
+
 int create_file(const char *filename, const mode_t mode);
 int create_name(char *buf, size_t size, const char *path,
 				const char *address, const char *name);
diff --git a/tools/csr_usb.c b/tools/csr_usb.c
index a483bc1..5fb6bdc 100644
--- a/tools/csr_usb.c
+++ b/tools/csr_usb.c
@@ -33,6 +33,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <dirent.h>
+#include <limits.h>
 #include <sys/ioctl.h>
 
 #include "csr.h"
diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index 95b4abf..2dbfca7 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -35,6 +35,7 @@
 #include <string.h>
 #include <dirent.h>
 #include <getopt.h>
+#include <limits.h>
 #include <sys/ioctl.h>
 #include <linux/types.h>
 #include <linux/hiddev.h>
-- 
1.8.5.3

Re: [PATCH v2 1/3] various header include fixes for building with musl libc

[Anderson Lizardo]

Hi Natanael,

On Wed, Jan 22, 2014 at 9:50 AM, Natanael Copa <natanael.copa@gmail.com> wrote:
> diff --git a/src/textfile.h b/src/textfile.h
> index b779bd2..e26da5d 100644
> --- a/src/textfile.h
> +++ b/src/textfile.h
> @@ -24,6 +24,8 @@
>  #ifndef __TEXTFILE_H
>  #define __TEXTFILE_H
>
> +#include <sys/stat.h>
> +

I believe the correct approach here is to include sys/stat.h on all
files that include textfile.h. We (usually) don't #include system
headers inside internal headers.

>  int create_file(const char *filename, const mode_t mode);
>  int create_name(char *buf, size_t size, const char *path,
>                                 const char *address, const char *name);

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

Re: [PATCH v2 1/3] various header include fixes for building with musl libc

[Marcel Holtmann]

Hi Anderson,

>> diff --git a/src/textfile.h b/src/textfile.h
>> index b779bd2..e26da5d 100644
>> --- a/src/textfile.h
>> +++ b/src/textfile.h
>> @@ -24,6 +24,8 @@
>> #ifndef __TEXTFILE_H
>> #define __TEXTFILE_H
>> 
>> +#include <sys/stat.h>
>> +
> 
> I believe the correct approach here is to include sys/stat.h on all
> files that include textfile.h. We (usually) don't #include system
> headers inside internal headers.

with code in src/shared/*.h we started to include the system headers that are required from the header, but you are correct, we don’t do that for internal src/*.h headers.

Also we do not use circular inclusion protection from internal headers. So I ripped the stupid __TEXTFILE_H stuff out now.

Regards

Marcel

Re: [PATCH v2 1/3] various header include fixes for building with musl libc

[Natanael Copa]

On Wed, 22 Jan 2014 11:16:20 -0400
Anderson Lizardo <anderson.lizardo@openbossa.org> wrote:

> Hi Natanael,
> 
> On Wed, Jan 22, 2014 at 9:50 AM, Natanael Copa <natanael.copa@gmail.com> wrote:
> > diff --git a/src/textfile.h b/src/textfile.h
> > index b779bd2..e26da5d 100644
> > --- a/src/textfile.h
> > +++ b/src/textfile.h
> > @@ -24,6 +24,8 @@
> >  #ifndef __TEXTFILE_H
> >  #define __TEXTFILE_H
> >
> > +#include <sys/stat.h>
> > +
> 
> I believe the correct approach here is to include sys/stat.h on all
> files that include textfile.h. We (usually) don't #include system
> headers inside internal headers.

The header itself uses mode_t:
./src/textfile.h:27:1: error: unknown type name 'mode_t'

So all the files that include textfiles.h needs to include sys/stat.h
*before* the include textfile.h in that case.

I'd say that all filesm that uses mode_t including textfile.h should
include sys/stat.h.

I can make a new patch for either. Just let me know what you want.


-nc

Re: [PATCH v2 1/3] various header include fixes for building with musl libc

[Anderson Lizardo]

Hi Natanael,

On Wed, Jan 22, 2014 at 12:07 PM, Natanael Copa <natanael.copa@gmail.com> wrote:
> The header itself uses mode_t:
> ./src/textfile.h:27:1: error: unknown type name 'mode_t'
>
> So all the files that include textfiles.h needs to include sys/stat.h
> *before* the include textfile.h in that case.
>
> I'd say that all filesm that uses mode_t including textfile.h should
> include sys/stat.h.
>
> I can make a new patch for either. Just let me know what you want.

This is how we have done so far for files in src/* (for src/shared/ as
Marcel mentioned this is being done differently). So I suggest you do
this way.

Note that several files (about 5 out of 10) already include
sys/stat.h, according to a quick grep I did. Maybe they are just not
in the right order.

Best Regards,
-- 
Anderson Lizardo
INdT - Manaus - Brazil

[PATCH v3] various header include fixes for building with musl libc

[Natanael Copa]

we need:
 sys/stat.h for mode_t
 limits.h for PATH_MAX

Fixes compile errors:
In file included from tools/hciconfig.c:45:0:
./src/textfile.h:27:1: error: unknown type name 'mode_t'
 int create_file(const char *filename, const mode_t mode);
 ^

tools/csr_usb.c: In function 'read_value':
tools/csr_usb.c:71:12: error: 'PATH_MAX' undeclared (first use in this function)
  char path[PATH_MAX];
            ^
---
Changes v2 -> v3:
 - include the sys/stat.h in hciconfig.c instead of textfile.h. This was
   the only needed change for defining mode_t everywhere.

 tools/csr_usb.c   | 1 +
 tools/hciconfig.c | 1 +
 tools/hid2hci.c   | 1 +
 3 files changed, 3 insertions(+)

diff --git a/tools/csr_usb.c b/tools/csr_usb.c
index a483bc1..5fb6bdc 100644
--- a/tools/csr_usb.c
+++ b/tools/csr_usb.c
@@ -33,6 +33,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <dirent.h>
+#include <limits.h>
 #include <sys/ioctl.h>
 
 #include "csr.h"
diff --git a/tools/hciconfig.c b/tools/hciconfig.c
index 6c7f8ed..a81dc9c 100644
--- a/tools/hciconfig.c
+++ b/tools/hciconfig.c
@@ -37,6 +37,7 @@
 #include <sys/param.h>
 #include <sys/ioctl.h>
 #include <sys/socket.h>
+#include <sys/stat.h>
 
 #include <bluetooth/bluetooth.h>
 #include <bluetooth/hci.h>
diff --git a/tools/hid2hci.c b/tools/hid2hci.c
index 95b4abf..2dbfca7 100644
--- a/tools/hid2hci.c
+++ b/tools/hid2hci.c
@@ -35,6 +35,7 @@
 #include <string.h>
 #include <dirent.h>
 #include <getopt.h>
+#include <limits.h>
 #include <sys/ioctl.h>
 #include <linux/types.h>
 #include <linux/hiddev.h>
-- 
1.8.5.3

Re: [PATCH v3] various header include fixes for building with musl libc

[Johan Hedberg]

Hi Natanael,

On Wed, Jan 22, 2014, Natanael Copa wrote:
> we need:
>  sys/stat.h for mode_t
>  limits.h for PATH_MAX
> 
> Fixes compile errors:
> In file included from tools/hciconfig.c:45:0:
> ./src/textfile.h:27:1: error: unknown type name 'mode_t'
>  int create_file(const char *filename, const mode_t mode);
>  ^
> 
> tools/csr_usb.c: In function 'read_value':
> tools/csr_usb.c:71:12: error: 'PATH_MAX' undeclared (first use in this function)
>   char path[PATH_MAX];
>             ^
> ---
> Changes v2 -> v3:
>  - include the sys/stat.h in hciconfig.c instead of textfile.h. This was
>    the only needed change for defining mode_t everywhere.
> 
>  tools/csr_usb.c   | 1 +
>  tools/hciconfig.c | 1 +
>  tools/hid2hci.c   | 1 +
>  3 files changed, 3 insertions(+)

Seems this patch was forgotten. It has now finally been applied
upstream.

Johan

[PATCH v2 2/3] bnep: avoid use of caddr_t

[Natanael Copa]

caddr_t is legacy BSD and should be avoided.

This fixes the following compile error with musl libc:
profiles/network/bnep.c: In function 'bnep_if_up':
profiles/network/bnep.c:205:33: error: 'caddr_t' undeclared (first use in this function)
  err = ioctl(sk, SIOCSIFFLAGS, (caddr_t) &ifr);
---
 profiles/network/bnep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/profiles/network/bnep.c b/profiles/network/bnep.c
index 2a74016..4f9b801 100644
--- a/profiles/network/bnep.c
+++ b/profiles/network/bnep.c
@@ -202,7 +202,7 @@ static int bnep_if_up(const char *devname)
 	ifr.ifr_flags |= IFF_UP;
 	ifr.ifr_flags |= IFF_MULTICAST;
 
-	err = ioctl(sk, SIOCSIFFLAGS, (caddr_t) &ifr);
+	err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
 
 	close(sk);
 
@@ -227,7 +227,7 @@ static int bnep_if_down(const char *devname)
 	ifr.ifr_flags &= ~IFF_UP;
 
 	/* Bring down the interface */
-	err = ioctl(sk, SIOCSIFFLAGS, (caddr_t) &ifr);
+	err = ioctl(sk, SIOCSIFFLAGS, (void *) &ifr);
 
 	close(sk);
 
-- 
1.8.5.3

[PATCH v2 3/3] unit: prevent use of glibc's error(3)

[Natanael Copa]

When building the test-sdp we don't want src/sdpd-request.c end up
using the incompatible GNU libc's error(3).

This also fixes the following compile error with musl libc which
misses the error(3) GNU extension:

src/sdpd-request.o: In function `extract_des':
/home/ncopa/src/bluez/src/sdpd-request.c:126: undefined reference to `error'
src/sdpd-request.o: In function `process_request':
/home/ncopa/src/bluez/src/sdpd-request.c:1022: undefined reference to `error'
/home/ncopa/src/bluez/src/sdpd-request.c:1045: undefined reference to `error'
---
 Makefile.am     | 1 +
 unit/test-sdp.c | 9 +++------
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 917f545..a05cacc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -251,6 +251,7 @@ unit_tests += unit/test-sdp
 unit_test_sdp_SOURCES = unit/test-sdp.c \
 				src/shared/util.h src/shared/util.c \
 				src/sdpd.h src/sdpd-database.c \
+				src/log.h src/log.c \
 				src/sdpd-service.c src/sdpd-request.c
 unit_test_sdp_LDADD = lib/libbluetooth-internal.la @GLIB_LIBS@
 
diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index 6d699e2..ba0e637 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -128,12 +128,6 @@ static void sdp_debug(const char *str, void *user_data)
 	g_print("%s%s\n", prefix, str);
 }
 
-void btd_debug(const char *format, ...);
-
-void btd_debug(const char *format, ...)
-{
-}
-
 static void context_quit(struct context *context)
 {
 	g_main_loop_quit(context->main_loop);
@@ -797,6 +791,9 @@ int main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
 
+	if (g_test_verbose())
+		__btd_log_init("*", 0);
+
 	/*
 	 * Service Search Request
 	 *
-- 
1.8.5.3