[PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Geert Uytterhoeven]

net/bluetooth/mgmt.c: In function ‘read_local_oob_ext_data_complete’:
net/bluetooth/mgmt.c:6474: warning: ‘r256’ may be used uninitialized in this function
net/bluetooth/mgmt.c:6474: warning: ‘h256’ may be used uninitialized in this function
net/bluetooth/mgmt.c:6474: warning: ‘r192’ may be used uninitialized in this function
net/bluetooth/mgmt.c:6474: warning: ‘h192’ may be used uninitialized in this function

While these are false positives, the code can be shortened by
pre-initializing the hash table pointers and eir_len. This has the side
effect of killing the compiler warnings.

Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
 net/bluetooth/mgmt.c | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 7fd87e7135b52753..f8e13b6d58279463 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6471,9 +6471,9 @@ static void read_local_oob_ext_data_complete(struct hci_dev *hdev, u8 status,
 {
 	const struct mgmt_cp_read_local_oob_ext_data *mgmt_cp;
 	struct mgmt_rp_read_local_oob_ext_data *mgmt_rp;
-	u8 *h192, *r192, *h256, *r256;
+	u8 *h192 = NULL, *r192 = NULL, *h256 = NULL, *r256 = NULL;
 	struct mgmt_pending_cmd *cmd;
-	u16 eir_len;
+	u16 eir_len = 0;
 	int err;
 
 	BT_DBG("%s status %u", hdev->name, status);
@@ -6486,18 +6486,11 @@ static void read_local_oob_ext_data_complete(struct hci_dev *hdev, u8 status,
 
 	if (status) {
 		status = mgmt_status(status);
-		eir_len = 0;
-
-		h192 = NULL;
-		r192 = NULL;
-		h256 = NULL;
-		r256 = NULL;
 	} else if (opcode == HCI_OP_READ_LOCAL_OOB_DATA) {
 		struct hci_rp_read_local_oob_data *rp;
 
 		if (skb->len != sizeof(*rp)) {
 			status = MGMT_STATUS_FAILED;
-			eir_len = 0;
 		} else {
 			status = MGMT_STATUS_SUCCESS;
 			rp = (void *)skb->data;
@@ -6505,23 +6498,18 @@ static void read_local_oob_ext_data_complete(struct hci_dev *hdev, u8 status,
 			eir_len = 5 + 18 + 18;
 			h192 = rp->hash;
 			r192 = rp->rand;
-			h256 = NULL;
-			r256 = NULL;
 		}
 	} else {
 		struct hci_rp_read_local_oob_ext_data *rp;
 
 		if (skb->len != sizeof(*rp)) {
 			status = MGMT_STATUS_FAILED;
-			eir_len = 0;
 		} else {
 			status = MGMT_STATUS_SUCCESS;
 			rp = (void *)skb->data;
 
 			if (hci_dev_test_flag(hdev, HCI_SC_ONLY)) {
 				eir_len = 5 + 18 + 18;
-				h192 = NULL;
-				r192 = NULL;
 			} else {
 				eir_len = 5 + 18 + 18 + 18 + 18;
 				h192 = rp->hash192;
-- 
1.9.1

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Marcel Holtmann]

Hi Geert,

> net/bluetooth/mgmt.c: In function ‘read_local_oob_ext_data_complete’:
> net/bluetooth/mgmt.c:6474: warning: ‘r256’ may be used uninitialized in this function
> net/bluetooth/mgmt.c:6474: warning: ‘h256’ may be used uninitialized in this function
> net/bluetooth/mgmt.c:6474: warning: ‘r192’ may be used uninitialized in this function
> net/bluetooth/mgmt.c:6474: warning: ‘h192’ may be used uninitialized in this function
> 
> While these are false positives, the code can be shortened by
> pre-initializing the hash table pointers and eir_len. This has the side
> effect of killing the compiler warnings.

can you be a bit specific on which compiler version is this. I fixed one occurrence that seemed valid. However in this case the compiler seems to be just plain stupid. On a gcc 4.9, I am not seeing these for example.

Regards

Marcel

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Geert Uytterhoeven]

Hi Marcel,

On Thu, Apr 16, 2015 at 10:34 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> net/bluetooth/mgmt.c: In function ‘read_local_oob_ext_data_complete’:
>> net/bluetooth/mgmt.c:6474: warning: ‘r256’ may be used uninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: ‘h256’ may be used uninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: ‘r192’ may be used uninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: ‘h192’ may be used uninitialized in this function
>>
>> While these are false positives, the code can be shortened by
>> pre-initializing the hash table pointers and eir_len. This has the side
>> effect of killing the compiler warnings.
>
> can you be a bit specific on which compiler version is this. I fixed one occurrence that seemed valid. However in this case the compiler seems to be just plain stupid. On a gcc 4.9, I am not seeing these for example.

gcc 4.1.2. As there were too many false positives, these warnings were
disabled in later versions (throwing away the children with the bad water).

If you don't like my patch, just drop it. I only look at newly
introduced warnings
of this kind anyway.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Geert Uytterhoeven]

Hi Marcel,

On Thu, Apr 16, 2015 at 10:34 PM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
>> net/bluetooth/mgmt.c: In function ‘read_local_oob_ext_data_complete’:
>> net/bluetooth/mgmt.c:6474: warning: ‘r256’ may be used uninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: ‘h256’ may be used uninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: ‘r192’ may be used uninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: ‘h192’ may be used uninitialized in this function
>>
>> While these are false positives, the code can be shortened by
>> pre-initializing the hash table pointers and eir_len. This has the side
>> effect of killing the compiler warnings.
>
> can you be a bit specific on which compiler version is this. I fixed one occurrence that seemed valid. However in this case the compiler seems to be just plain stupid. On a gcc 4.9, I am not seeing these for example.

gcc 4.1.2. As there were too many false positives, these warnings were
disabled in later versions (throwing away the children with the bad water).

If you don't like my patch, just drop it. I only look at newly
introduced warnings
of this kind anyway.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Geert Uytterhoeven]

Hi Marcel,

On Thu, Apr 16, 2015 at 10:34 PM, Marcel Holtmann <marcel@holtmann.org> wro=
te:
>> net/bluetooth/mgmt.c: In function =E2=80=98read_local_oob_ext_data_compl=
ete=E2=80=99:
>> net/bluetooth/mgmt.c:6474: warning: =E2=80=98r256=E2=80=99 may be used u=
ninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: =E2=80=98h256=E2=80=99 may be used u=
ninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: =E2=80=98r192=E2=80=99 may be used u=
ninitialized in this function
>> net/bluetooth/mgmt.c:6474: warning: =E2=80=98h192=E2=80=99 may be used u=
ninitialized in this function
>>
>> While these are false positives, the code can be shortened by
>> pre-initializing the hash table pointers and eir_len. This has the side
>> effect of killing the compiler warnings.
>
> can you be a bit specific on which compiler version is this. I fixed one =
occurrence that seemed valid. However in this case the compiler seems to be=
 just plain stupid. On a gcc 4.9, I am not seeing these for example.

gcc 4.1.2. As there were too many false positives, these warnings were
disabled in later versions (throwing away the children with the bad water).

If you don't like my patch, just drop it. I only look at newly
introduced warnings
of this kind anyway.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org

In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
                                -- Linus Torvalds

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Marcel Holtmann]

Hi Geert,

>>> net/bluetooth/mgmt.c: In function ‘read_local_oob_ext_data_complete’:
>>> net/bluetooth/mgmt.c:6474: warning: ‘r256’ may be used uninitialized in this function
>>> net/bluetooth/mgmt.c:6474: warning: ‘h256’ may be used uninitialized in this function
>>> net/bluetooth/mgmt.c:6474: warning: ‘r192’ may be used uninitialized in this function
>>> net/bluetooth/mgmt.c:6474: warning: ‘h192’ may be used uninitialized in this function
>>> 
>>> While these are false positives, the code can be shortened by
>>> pre-initializing the hash table pointers and eir_len. This has the side
>>> effect of killing the compiler warnings.
>> 
>> can you be a bit specific on which compiler version is this. I fixed one occurrence that seemed valid. However in this case the compiler seems to be just plain stupid. On a gcc 4.9, I am not seeing these for example.
> 
> gcc 4.1.2. As there were too many false positives, these warnings were
> disabled in later versions (throwing away the children with the bad water).
> 
> If you don't like my patch, just drop it. I only look at newly
> introduced warnings
> of this kind anyway.

I really do not know what is the best solution here. This is a false positive. And I have been looking at this particular code for a warning that was valid, but we missed initially. But these warnings that you are fixing are clearly false positive.

If this only happens with an old compiler version, I would tend to leave the code as is. Then again, what is the general preferred approach here?

Regards

Marcel

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Marcel Holtmann]

Hi Geert,

>>> net/bluetooth/mgmt.c: In function ‘read_local_oob_ext_data_complete’:
>>> net/bluetooth/mgmt.c:6474: warning: ‘r256’ may be used uninitialized in this function
>>> net/bluetooth/mgmt.c:6474: warning: ‘h256’ may be used uninitialized in this function
>>> net/bluetooth/mgmt.c:6474: warning: ‘r192’ may be used uninitialized in this function
>>> net/bluetooth/mgmt.c:6474: warning: ‘h192’ may be used uninitialized in this function
>>> 
>>> While these are false positives, the code can be shortened by
>>> pre-initializing the hash table pointers and eir_len. This has the side
>>> effect of killing the compiler warnings.
>> 
>> can you be a bit specific on which compiler version is this. I fixed one occurrence that seemed valid. However in this case the compiler seems to be just plain stupid. On a gcc 4.9, I am not seeing these for example.
> 
> gcc 4.1.2. As there were too many false positives, these warnings were
> disabled in later versions (throwing away the children with the bad water).
> 
> If you don't like my patch, just drop it. I only look at newly
> introduced warnings
> of this kind anyway.

I really do not know what is the best solution here. This is a false positive. And I have been looking at this particular code for a warning that was valid, but we missed initially. But these warnings that you are fixing are clearly false positive.

If this only happens with an old compiler version, I would tend to leave the code as is. Then again, what is the general preferred approach here?

Regards

Marcel

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Geert Uytterhoeven]

Hi Marcel,

On Fri, Apr 17, 2015 at 10:38 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>>>> net/bluetooth/mgmt.c: In function ‘read_local_oob_ext_data_complete’:
>>>> net/bluetooth/mgmt.c:6474: warning: ‘r256’ may be used uninitialized in this function
>>>> net/bluetooth/mgmt.c:6474: warning: ‘h256’ may be used uninitialized in this function
>>>> net/bluetooth/mgmt.c:6474: warning: ‘r192’ may be used uninitialized in this function
>>>> net/bluetooth/mgmt.c:6474: warning: ‘h192’ may be used uninitialized in this function
>>>>
>>>> While these are false positives, the code can be shortened by
>>>> pre-initializing the hash table pointers and eir_len. This has the side
>>>> effect of killing the compiler warnings.
>>>
>>> can you be a bit specific on which compiler version is this. I fixed one occurrence that seemed valid. However in this case the compiler seems to be just plain stupid. On a gcc 4.9, I am not seeing these for example.
>>
>> gcc 4.1.2. As there were too many false positives, these warnings were
>> disabled in later versions (throwing away the children with the bad water).
>>
>> If you don't like my patch, just drop it. I only look at newly
>> introduced warnings
>> of this kind anyway.
>
> I really do not know what is the best solution here. This is a false positive. And I have been looking at this particular code for a warning that was valid, but we missed initially. But these warnings that you are fixing are clearly false positive.

I only sent patches to fix false positives if I think the patches improve the
code. As this is a subjective matter, it's up to you as the maintainer to
decide.

> If this only happens with an old compiler version, I would tend to leave the code as is. Then again, what is the general preferred approach here?

As this is a false positive, it's clearly up to the maintainer to
decide if the patch
improves the code or not.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Geert Uytterhoeven]

Hi Marcel,

On Fri, Apr 17, 2015 at 10:38 PM, Marcel Holtmann <marcel@holtmann.org> wro=
te:
>>>> net/bluetooth/mgmt.c: In function =E2=80=98read_local_oob_ext_data_com=
plete=E2=80=99:
>>>> net/bluetooth/mgmt.c:6474: warning: =E2=80=98r256=E2=80=99 may be used=
 uninitialized in this function
>>>> net/bluetooth/mgmt.c:6474: warning: =E2=80=98h256=E2=80=99 may be used=
 uninitialized in this function
>>>> net/bluetooth/mgmt.c:6474: warning: =E2=80=98r192=E2=80=99 may be used=
 uninitialized in this function
>>>> net/bluetooth/mgmt.c:6474: warning: =E2=80=98h192=E2=80=99 may be used=
 uninitialized in this function
>>>>
>>>> While these are false positives, the code can be shortened by
>>>> pre-initializing the hash table pointers and eir_len. This has the sid=
e
>>>> effect of killing the compiler warnings.
>>>
>>> can you be a bit specific on which compiler version is this. I fixed on=
e occurrence that seemed valid. However in this case the compiler seems to =
be just plain stupid. On a gcc 4.9, I am not seeing these for example.
>>
>> gcc 4.1.2. As there were too many false positives, these warnings were
>> disabled in later versions (throwing away the children with the bad wate=
r).
>>
>> If you don't like my patch, just drop it. I only look at newly
>> introduced warnings
>> of this kind anyway.
>
> I really do not know what is the best solution here. This is a false posi=
tive. And I have been looking at this particular code for a warning that wa=
s valid, but we missed initially. But these warnings that you are fixing ar=
e clearly false positive.

I only sent patches to fix false positives if I think the patches improve t=
he
code. As this is a subjective matter, it's up to you as the maintainer to
decide.

> If this only happens with an old compiler version, I would tend to leave =
the code as is. Then again, what is the general preferred approach here?

As this is a false positive, it's clearly up to the maintainer to
decide if the patch
improves the code or not.

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k=
.org

In personal conversations with technical people, I call myself a hacker. Bu=
t
when I'm talking to journalists I just say "programmer" or something like t=
hat.
                                -- Linus Torvalds

Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete()

[Marcel Holtmann]

Hi Geert,

>>>>> net/bluetooth/mgmt.c: In function ‘read_local_oob_ext_data_complete’:
>>>>> net/bluetooth/mgmt.c:6474: warning: ‘r256’ may be used uninitialized in this function
>>>>> net/bluetooth/mgmt.c:6474: warning: ‘h256’ may be used uninitialized in this function
>>>>> net/bluetooth/mgmt.c:6474: warning: ‘r192’ may be used uninitialized in this function
>>>>> net/bluetooth/mgmt.c:6474: warning: ‘h192’ may be used uninitialized in this function
>>>>> 
>>>>> While these are false positives, the code can be shortened by
>>>>> pre-initializing the hash table pointers and eir_len. This has the side
>>>>> effect of killing the compiler warnings.
>>>> 
>>>> can you be a bit specific on which compiler version is this. I fixed one occurrence that seemed valid. However in this case the compiler seems to be just plain stupid. On a gcc 4.9, I am not seeing these for example.
>>> 
>>> gcc 4.1.2. As there were too many false positives, these warnings were
>>> disabled in later versions (throwing away the children with the bad water).
>>> 
>>> If you don't like my patch, just drop it. I only look at newly
>>> introduced warnings
>>> of this kind anyway.
>> 
>> I really do not know what is the best solution here. This is a false positive. And I have been looking at this particular code for a warning that was valid, but we missed initially. But these warnings that you are fixing are clearly false positive.
> 
> I only sent patches to fix false positives if I think the patches improve the
> code. As this is a subjective matter, it's up to you as the maintainer to
> decide.
> 
>> If this only happens with an old compiler version, I would tend to leave the code as is. Then again, what is the general preferred approach here?
> 
> As this is a false positive, it's clearly up to the maintainer to
> decide if the patch
> improves the code or not.

and in this case, I have no idea if I want to bother or not. I really just don’t. Since nobody else complained, I might just leave it as is since it really is a false positive.

Regards

Marcel