From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754607AbbDTIrH (ORCPT ); Mon, 20 Apr 2015 04:47:07 -0400 Received: from mail-oi0-f44.google.com ([209.85.218.44]:33699 "EHLO mail-oi0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753487AbbDTIrE convert rfc822-to-8bit (ORCPT ); Mon, 20 Apr 2015 04:47:04 -0400 MIME-Version: 1.0 In-Reply-To: <8684664B-22C2-45E8-B547-48F173B3CA7A@holtmann.org> References: <1429212265-26750-1-git-send-email-geert@linux-m68k.org> <8684664B-22C2-45E8-B547-48F173B3CA7A@holtmann.org> Date: Mon, 20 Apr 2015 10:47:03 +0200 X-Google-Sender-Auth: eftUEBnVSyn1JYFb1jV6_DKE8V8 Message-ID: Subject: Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete() From: Geert Uytterhoeven To: Marcel Holtmann Cc: "Gustavo F. Padovan" , Johan Hedberg , BlueZ development , netdev , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Marcel, On Fri, Apr 17, 2015 at 10:38 PM, Marcel Holtmann 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: MIME-Version: 1.0 Sender: geert.uytterhoeven@gmail.com In-Reply-To: <8684664B-22C2-45E8-B547-48F173B3CA7A@holtmann.org> References: <1429212265-26750-1-git-send-email-geert@linux-m68k.org> <8684664B-22C2-45E8-B547-48F173B3CA7A@holtmann.org> Date: Mon, 20 Apr 2015 10:47:03 +0200 Message-ID: Subject: Re: [PATCH] Bluetooth: Pre-initialize variables in read_local_oob_ext_data_complete() From: Geert Uytterhoeven To: Marcel Holtmann Cc: "Gustavo F. Padovan" , Johan Hedberg , BlueZ development , netdev , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 List-ID: Hi Marcel, On Fri, Apr 17, 2015 at 10:38 PM, Marcel Holtmann 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