2018-05-09 12:11:11

by Szymon Janc

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

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



2018-05-25 08:03:33

by Szymon Janc

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

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



2018-05-09 13:04:16

by Szymon Janc

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

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

2018-05-09 12:26:36

by Marcel Holtmann

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

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


2018-05-09 12:11:13

by Szymon Janc

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

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


2018-05-09 12:11:12

by Szymon Janc

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

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