From mboxrd@z Thu Jan 1 00:00:00 1970 From: Colin Ian King Subject: Re: [PATCH] tty/metag_da: initialize number_written to zero Date: Sun, 7 Feb 2016 07:59:22 +0000 Message-ID: <56B6F95A.7040003@canonical.com> References: <1453851445-4645-1-git-send-email-colin.king@canonical.com> <20160127114206.GB20777@jhogan-linux.le.imgtec.org> <56AA6271.1070108@canonical.com> <20160207074109.GA6508@kroah.com> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20160207074109.GA6508-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org> Sender: linux-metag-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: Content-Type: text/plain; charset="us-ascii" To: Greg Kroah-Hartman Cc: James Hogan , Jiri Slaby , linux-metag-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org On 07/02/16 07:41, Greg Kroah-Hartman wrote: > On Thu, Jan 28, 2016 at 06:48:17PM +0000, Colin Ian King wrote: >> On 27/01/16 11:42, James Hogan wrote: >>> Hi Colin, >>> >>> On Tue, Jan 26, 2016 at 11:37:25PM +0000, Colin King wrote: >>>> From: Colin Ian King >>>> >>>> number_written is not initialized, so it can be any value. In the >>>> case where dport->xmit_cnt is zero, number_written is not set >>>> and subsequent accesses to it will be reading a garbage value. >>> >>> the only subsequent accesses when dport->xmit_cnt == 0 are: >>> >>> /* if we've made more data available, wake up tty */ >>> if (count && number_written) { >>> >>> and: >>> >>> /* did the write fail? */ >>> return count && !number_written; >>> >>> but dport->xmit_cnt == 0 implies count == 0, so number_written shouldn >> 't >>> be used, and both will evaluate to false regardless of the uninitialis >> ed >>> value, so it looks fine as it is to me. >>> >>> Is this tripping up some static analysis tool or something? >> >> It was found using cppcheck, namely: >> >> [drivers/tty/metag_da.c:269]: (error) Uninitialized variable: number_wri >> tten > > Please fix the broken tool, don't paper over it by doing unnecessary > work in the kernel. > > thanks, > > greg k-h > Sorry Greg, this was my first thinko out of 80 or so fixes I've found with static analysis. I'll try harder next time not to make a mistake. Colin From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753564AbcBGH71 (ORCPT ); Sun, 7 Feb 2016 02:59:27 -0500 Received: from youngberry.canonical.com ([91.189.89.112]:41899 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751067AbcBGH70 (ORCPT ); Sun, 7 Feb 2016 02:59:26 -0500 Subject: Re: [PATCH] tty/metag_da: initialize number_written to zero To: Greg Kroah-Hartman References: <1453851445-4645-1-git-send-email-colin.king@canonical.com> <20160127114206.GB20777@jhogan-linux.le.imgtec.org> <56AA6271.1070108@canonical.com> <20160207074109.GA6508@kroah.com> Cc: James Hogan , Jiri Slaby , linux-metag@vger.kernel.org, linux-kernel@vger.kernel.org From: Colin Ian King Message-ID: <56B6F95A.7040003@canonical.com> Date: Sun, 7 Feb 2016 07:59:22 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20160207074109.GA6508@kroah.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/02/16 07:41, Greg Kroah-Hartman wrote: > On Thu, Jan 28, 2016 at 06:48:17PM +0000, Colin Ian King wrote: >> On 27/01/16 11:42, James Hogan wrote: >>> Hi Colin, >>> >>> On Tue, Jan 26, 2016 at 11:37:25PM +0000, Colin King wrote: >>>> From: Colin Ian King >>>> >>>> number_written is not initialized, so it can be any value. In the >>>> case where dport->xmit_cnt is zero, number_written is not set >>>> and subsequent accesses to it will be reading a garbage value. >>> >>> the only subsequent accesses when dport->xmit_cnt == 0 are: >>> >>> /* if we've made more data available, wake up tty */ >>> if (count && number_written) { >>> >>> and: >>> >>> /* did the write fail? */ >>> return count && !number_written; >>> >>> but dport->xmit_cnt == 0 implies count == 0, so number_written shouldn >> 't >>> be used, and both will evaluate to false regardless of the uninitialis >> ed >>> value, so it looks fine as it is to me. >>> >>> Is this tripping up some static analysis tool or something? >> >> It was found using cppcheck, namely: >> >> [drivers/tty/metag_da.c:269]: (error) Uninitialized variable: number_wri >> tten > > Please fix the broken tool, don't paper over it by doing unnecessary > work in the kernel. > > thanks, > > greg k-h > Sorry Greg, this was my first thinko out of 80 or so fixes I've found with static analysis. I'll try harder next time not to make a mistake. Colin