[PATCH 1/3] build: Disable some warnings from GCC 8

[PATCH 1/3] build: Disable some warnings from GCC 8

[Szymon Janc]

GCC 8 -Wall -Wextra seem to enable additional checks. Those generates
lots of spourious warnings so disable them (at least for time being).
---
 acinclude.m4 | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/acinclude.m4 b/acinclude.m4
index bc39c6d73..39a6be44a 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -16,6 +16,9 @@ AC_DEFUN([COMPILER_FLAGS], [
 		with_cflags="$with_cflags -Wall -Werror -Wextra"
 		with_cflags="$with_cflags -Wno-unused-parameter"
 		with_cflags="$with_cflags -Wno-missing-field-initializers"
+		with_cflags="$with_cflags -Wno-format-truncation"
+		with_cflags="$with_cflags -Wno-cast-function-type"
+		with_cflags="$with_cflags -Wno-stringop-truncation"
 		with_cflags="$with_cflags -Wdeclaration-after-statement"
 		with_cflags="$with_cflags -Wmissing-declarations"
 		with_cflags="$with_cflags -Wredundant-decls"
-- 
2.17.0

[PATCH 2/3] profiles/midi: Fix compilation with GCC 8

[Szymon Janc]

We can use strcpy here since addr is smaller.

  CC       profiles/midi/bluetoothd-midi.o
profiles/midi/midi.c: In function ‘midi_accept’:
profiles/midi/midi.c:307:36: error: argument to ‘sizeof’
       in ‘strncpy’ call is the same expression as the source;
       did you mean to use the size of the destination?
       [-Werror=sizeof-pointer-memaccess]
   strncpy(device_name, addr, sizeof(addr));
                                    ^
cc1: all warnings being treated as errors
---
 profiles/midi/midi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/profiles/midi/midi.c b/profiles/midi/midi.c
index fdc1c007d..9e04d19e3 100644
--- a/profiles/midi/midi.c
+++ b/profiles/midi/midi.c
@@ -304,7 +304,7 @@ static int midi_accept(struct btd_service *service)
 	if (device_name_known(device))
 		device_get_name(device, device_name, sizeof(device_name));
 	else
-		strncpy(device_name, addr, sizeof(addr));
+		strcpy(device_name, addr);
 
 	/* ALSA Sequencer Client and Port Setup */
 	err = snd_seq_open(&midi->seq_handle, "default", SND_SEQ_OPEN_DUPLEX, 0);
-- 
2.17.0

[PATCH 3/3] android/client: Fix compilation with GCC 8

[Szymon Janc]

  CC       android/client/android_haltest-if-gatt.o
android/client/if-gatt.c: In function ‘multi_adv_set_inst_data_p’:
android/client/if-gatt.c:2034:7: error: ‘set_scan_rsp’ may be used
      uninitialized in this function [-Werror=maybe-uninitialized]
  EXEC(if_gatt->client->multi_adv_set_inst_data, client_if, set_scan_rsp,
       ^~~~~~~
android/client/if-gatt.c:2034:7: error: ‘include_name’ may be used
         uninitialized in this function [-Werror=maybe-uninitialized]
android/client/if-gatt.c:2034:7: error: ‘include_txpower’ may be used
         uninitialized in this function [-Werror=maybe-uninitialized]
android/client/if-gatt.c: In function ‘set_adv_data_p’:
android/client/if-gatt.c:1859:7: error: ‘set_scan_rsp’ may be used
         uninitialized in this function [-Werror=maybe-uninitialized]
  EXEC(if_gatt->client->set_adv_data, client_if, set_scan_rsp,
       ^~~~~~~
android/client/if-gatt.c:1859:7: error: ‘include_name’ may be used
         uninitialized in this function [-Werror=maybe-uninitialized]
android/client/if-gatt.c:1859:7: error: ‘include_txpower’ may be used
         uninitialized in this function [-Werror=maybe-uninitialized]
cc1: all warnings being treated as errors
make[1]: *** [Makefile:6431: android/client/android_haltest-if-gatt.o] Error 1
make: *** [Makefile:3262: all] Error 2
---
 android/client/if-gatt.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/android/client/if-gatt.c b/android/client/if-gatt.c
index 35267832f..ed14f92c4 100644
--- a/android/client/if-gatt.c
+++ b/android/client/if-gatt.c
@@ -1825,8 +1825,9 @@ static void set_adv_data_c(int argc, const char **argv,
 static void set_adv_data_p(int argc, const char **argv)
 {
 	int client_if;
-	bool set_scan_rsp;
-	bool include_name, include_txpower;
+	bool set_scan_rsp = false;
+	bool include_name = false;
+	bool include_txpower = false;
 	int min_interval, max_interval;
 	int appearance;
 	uint16_t manufacturer_len;
@@ -2003,8 +2004,9 @@ static void multi_adv_set_inst_data_c(int argc, const char **argv,
 static void multi_adv_set_inst_data_p(int argc, const char **argv)
 {
 	int client_if;
-	bool set_scan_rsp;
-	bool include_name, include_txpower;
+	bool set_scan_rsp = false;
+	bool include_name = false;
+	bool include_txpower = false;
 	int appearance;
 	uint16_t manufacturer_len;
 	uint8_t manufacturer_data[100];
-- 
2.17.0

Re: [PATCH 1/3] build: Disable some warnings from GCC 8

[Marcel Holtmann]

Hi Szymon,

> GCC 8 -Wall -Wextra seem to enable additional checks. Those generates
> lots of spourious warnings so disable them (at least for time being).
> ---
> acinclude.m4 | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index bc39c6d73..39a6be44a 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -16,6 +16,9 @@ AC_DEFUN([COMPILER_FLAGS], [
> 		with_cflags="$with_cflags -Wall -Werror -Wextra"
> 		with_cflags="$with_cflags -Wno-unused-parameter"
> 		with_cflags="$with_cflags -Wno-missing-field-initializers"
> +		with_cflags="$with_cflags -Wno-format-truncation"
> +		with_cflags="$with_cflags -Wno-cast-function-type"
> +		with_cflags="$with_cflags -Wno-stringop-truncation”

can you explain these with examples on why we would disable them.

Regards

Marcel

Re: [PATCH 1/3] build: Disable some warnings from GCC 8

[Szymon Janc]

Hi Marce,

On Wednesday, 9 May 2018 14:26:36 CEST Marcel Holtmann wrote:
> Hi Szymon,
>=20
> > GCC 8 -Wall -Wextra seem to enable additional checks. Those generates
> > lots of spourious warnings so disable them (at least for time being).
> > ---
> > acinclude.m4 | 3 +++
> > 1 file changed, 3 insertions(+)
> >=20
> > diff --git a/acinclude.m4 b/acinclude.m4
> > index bc39c6d73..39a6be44a 100644
> > --- a/acinclude.m4
> > +++ b/acinclude.m4
> > @@ -16,6 +16,9 @@ AC_DEFUN([COMPILER_FLAGS], [
> >=20
> > 		with_cflags=3D"$with_cflags -Wall -Werror -Wextra"
> > 		with_cflags=3D"$with_cflags -Wno-unused-parameter"
> > 		with_cflags=3D"$with_cflags -Wno-missing-field-initializers"
> >=20
> > +		with_cflags=3D"$with_cflags -Wno-format-truncation"
> > +		with_cflags=3D"$with_cflags -Wno-cast-function-type"
> > +		with_cflags=3D"$with_cflags -Wno-stringop-truncation=E2=80=9D
>=20
> can you explain these with examples on why we would disable them.

=2DWformat-truncation

Than one warns when snprintf could be truncated:
eg=20
char a[10];
char b[10];
char c[15];
=2E.
snprintf(c, sizeof(c), "%s%s", a, b);

This sometimes triggers warning also with "sizeof(c) - 1" when c is either=
=20
memset before or last byte being explicitly set to '\0' after snprintf.

Also, sometimes we are fine if resulting string is left unterminated eg
when filling in mgmt commands or struct sockaddr with paths and/or names.
Some could be change to memcpy + strlen (possibly after getting min size fr=
om=20
both buffers) or we could adjust buffers to make those go away.



=2DWstringop-truncation
This is pretty match the same as above but for strncpy and family.
Also here, some could be change to memcpy + strlen. Or even better don't us=
e=20
strncpy at all and have own implementation of strlcpy.



=2DWcast-function-type
Warn when a function pointer is cast to an incompatible function pointer.
We often cast function pointers with g_slist_foreach or queue call to avoid
need for having wrappers. This seems to be fine as along as casted function
doesn't expect more parameters than type it is casted to. To fix those we=20
would have to always wrap eg:

+static void update_adapter_cb(gpointer data,  gpointer user_data)
+{
+	struct hdp_adapter *hdp_adapter =3D data;
+
+	update_adapter(hdp_adapter);
+}
+
 static void remove_application(struct hdp_application *app)
 {
 	DBG("Application %s deleted", app->path);
 	hdp_application_unref(app);
=20
=2D	g_slist_foreach(adapters, (GFunc) update_adapter, NULL);
+	g_slist_foreach(adapters, update_adapter_cb, NULL);
 }



=2D-=20
pozdrawiam
Szymon Janc

Re: [PATCH 3/3] android/client: Fix compilation with GCC 8

[Szymon Janc]

On Wednesday, 9 May 2018 14:11:13 CEST Szymon Janc wrote:
>   CC       android/client/android_haltest-if-gatt.o
> android/client/if-gatt.c: In function =E2=80=98multi_adv_set_inst_data_p=
=E2=80=99:
> android/client/if-gatt.c:2034:7: error: =E2=80=98set_scan_rsp=E2=80=99 ma=
y be used
>       uninitialized in this function [-Werror=3Dmaybe-uninitialized]
>   EXEC(if_gatt->client->multi_adv_set_inst_data, client_if, set_scan_rsp,
>        ^~~~~~~
> android/client/if-gatt.c:2034:7: error: =E2=80=98include_name=E2=80=99 ma=
y be used
>          uninitialized in this function [-Werror=3Dmaybe-uninitialized]
> android/client/if-gatt.c:2034:7: error: =E2=80=98include_txpower=E2=80=99=
 may be used
>          uninitialized in this function [-Werror=3Dmaybe-uninitialized]
> android/client/if-gatt.c: In function =E2=80=98set_adv_data_p=E2=80=99:
> android/client/if-gatt.c:1859:7: error: =E2=80=98set_scan_rsp=E2=80=99 ma=
y be used
>          uninitialized in this function [-Werror=3Dmaybe-uninitialized]
>   EXEC(if_gatt->client->set_adv_data, client_if, set_scan_rsp,
>        ^~~~~~~
> android/client/if-gatt.c:1859:7: error: =E2=80=98include_name=E2=80=99 ma=
y be used
>          uninitialized in this function [-Werror=3Dmaybe-uninitialized]
> android/client/if-gatt.c:1859:7: error: =E2=80=98include_txpower=E2=80=99=
 may be used
>          uninitialized in this function [-Werror=3Dmaybe-uninitialized]
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:6431: android/client/android_haltest-if-gatt.o] Er=
ror
> 1 make: *** [Makefile:3262: all] Error 2
> ---
>  android/client/if-gatt.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>=20
> diff --git a/android/client/if-gatt.c b/android/client/if-gatt.c
> index 35267832f..ed14f92c4 100644
> --- a/android/client/if-gatt.c
> +++ b/android/client/if-gatt.c
> @@ -1825,8 +1825,9 @@ static void set_adv_data_c(int argc, const char
> **argv, static void set_adv_data_p(int argc, const char **argv)
>  {
>  	int client_if;
> -	bool set_scan_rsp;
> -	bool include_name, include_txpower;
> +	bool set_scan_rsp =3D false;
> +	bool include_name =3D false;
> +	bool include_txpower =3D false;
>  	int min_interval, max_interval;
>  	int appearance;
>  	uint16_t manufacturer_len;
> @@ -2003,8 +2004,9 @@ static void multi_adv_set_inst_data_c(int argc, con=
st
> char **argv, static void multi_adv_set_inst_data_p(int argc, const char
> **argv) {
>  	int client_if;
> -	bool set_scan_rsp;
> -	bool include_name, include_txpower;
> +	bool set_scan_rsp =3D false;
> +	bool include_name =3D false;
> +	bool include_txpower =3D false;
>  	int appearance;
>  	uint16_t manufacturer_len;
>  	uint8_t manufacturer_data[100];


This patch is now applied.

=2D-=20
pozdrawiam
Szymon Janc