From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Subject: Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types Date: Tue, 22 Sep 2015 11:13:29 +0200 Message-ID: <56011BB9.5030004@samsung.com> References: <1442842450-29769-1-git-send-email-a.hajda@samsung.com> <17571.1442842945@warthog.procyon.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-reply-to: <17571.1442842945@warthog.procyon.org.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: David Howells Cc: linux-mips@linux-mips.org, linux-fbdev@vger.kernel.org, linux-sh@vger.kernel.org, brcm80211-dev-list@broadcom.com, dri-devel@lists.freedesktop.org, linux-mm@kvack.org, linux-clk@vger.kernel.org, linux-leds@vger.kernel.org, Marek Szyprowski , devel@driverdev.osuosl.org, linux-rdma@vger.kernel.org, linux-cachefs@redhat.com, linux-serial@vger.kernel.org, linux-input@vger.kernel.org, linux-media@vger.kernel.org, dev@openvswitch.org, rtc-linux@googlegroups.com, Bartlomiej Zolnierkiewicz , intel-gfx@lists.freedesktop.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Andrzej Hajda , linux-api@vger.kernel.org, linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org, netdev@vger.kernel.org, lustre-devel@lists.lustre.org List-Id: linux-api@vger.kernel.org T24gMDkvMjEvMjAxNSAwMzo0MiBQTSwgRGF2aWQgSG93ZWxscyB3cm90ZToKPiBBbmRyemVqIEhh amRhIDxhLmhhamRhLVN6ZTNPM1VVMjJKQkRnaks3eTdUVVFAcHVibGljLmdtYW5lLm9yZz4gd3Jv dGU6Cj4gCj4+IFNlbWFudGljIHBhdGNoIGZpbmRzIGNvbXBhcmlzb25zIG9mIHR5cGVzOgo+PiAg ICAgdW5zaWduZWQgPCAwCj4+ICAgICB1bnNpZ25lZCA+PSAwCj4+IFRoZSBmb3JtZXIgaXMgYWx3 YXlzIGZhbHNlLCB0aGUgbGF0dGVyIGlzIGFsd2F5cyB0cnVlLgo+PiBTdWNoIGNvbXBhcmlzb25z IGFyZSB1c2VsZXNzLCBzbyB0aGVvcmV0aWNhbGx5IHRoZXkgY291bGQgYmUKPj4gc2FmZWx5IHJl bW92ZWQsIGJ1dCB0aGVpciBwcmVzZW5jZSBxdWl0ZSBvZnRlbiBpbmRpY2F0ZXMgYnVncy4KPiAK PiBPciBzb21lb25lIGhhcyBsZWZ0IHRoZW0gaW4gYmVjYXVzZSB0aGV5IGRvbid0IG1hdHRlciBh bmQgdGhlcmUncyB0aGUKPiBwb3NzaWJpbGl0eSB0aGF0IHRoZSB0eXBlIGJlaW5nIHRlc3RlZCBt aWdodCBiZSBvciBiZWNvbWUgc2lnbmVkIHVuZGVyIHNvbWUKPiBjaXJjdW1zdGFuY2VzLiAgSWYg dGhlIGNvbXBhcmlzb24gaXMgdXNlbGVzcywgSSdkIGV4cGVjdCB0aGUgY29tcGlsZXIgdG8ganVz dAo+IGRpc2NhcmQgaXQgLSBmb3Igc3VjaCBjYXNlcyB5b3VyIHBhdGNoIGlzIHBvaW50bGVzcy4K PiAKPiBJZiBJIGhhdmUsIGZvciBleGFtcGxlOgo+IAo+IAl1bnNpZ25lZCB4Owo+IAo+IAlpZiAo eCA9PSAwIHx8IHggPiAyNykKPiAJCWdpdmVfYV9yYW5nZV9lcnJvcigpOwo+IAo+IEkgd2lsbCB3 cml0ZSB0aGlzIGFzOgo+IAo+IAl1bnNpZ25lZCB4Owo+IAo+IAlpZiAoeCA8PSAwIHx8IHggPiAy NykKPiAJCWdpdmVfYV9yYW5nZV9lcnJvcigpOwo+IAo+IGJlY2F1c2UgaXQgdGhhdCBnaXZlcyBh IHdheSB0byBoYW5kbGUgeCBiZWluZyBjaGFuZ2VkIHRvIHNpZ25lZCBhdCBzb21lIHBvaW50Cj4g aW4gdGhlIGZ1dHVyZSBmb3Igbm8gY29zdC4gIEluIHdoaWNoIGNhc2UsIHlvdXIgY2hhbmdpbmcg dGhlIDw9IHRvIGFuID09Cj4gImJlY2F1c2UgdGhlIDwgcGFydCBvZiB0aGUgY2FzZSBpcyB1c2Vs ZXNzIiBpcyBhcmd1YWJseSB3cm9uZy4KClRoaXMgaXMgd2h5IEkgaGF2ZSBub3QgY2hlY2tlZCBm b3Igc3VjaCBjYXNlcyAtIEkgaGF2ZSBza2lwcGVkIGNoZWNrcyBvZiB0eXBlCgl1bnNpZ25lZCA8 PSAwCmV4YWN0bHkgZm9yIHRoZSByZWFzb25zIGFib3ZlLgoKSG93ZXZlciBJIGhhdmUgbGVmdCB0 d28gb3RoZXIgY2hlY2tzIGFzIHRoZXkgc2VlbXMgdG8gbWUgbW9yZSBzdXNwaWNpb3VzIC0gdGhl eQphcmUgYWx3YXlzIHRydWUgb3IgZmFsc2UuIEJ1dCBhcyBEbWl0cnkgYW5kIEFuZHJldyBwb2lu dGVkIG91dCBMaW51cyBoYXZlIHF1aXRlCnN0cm9uZyBvcGluaW9uIGFnYWluc3QgcmVtb3Zpbmcg cmFuZ2UgY2hlY2tzIGluIHN1Y2ggY2FzZXMgYXMgaGUgZmluZHMgaXQKY2xlYXJlci4gSSB0aGlu ayBpdCBhcHBsaWVzIHRvIHBhdGNoZXMgMjktMzYuIEkgYW0gbm90IHN1cmUgYWJvdXQgcGF0Y2hl cyAyNi0yOCwzNy4KClJlZ2FyZHMKQW5kcnplagoKPiAKPiBEYXZpZAo+IC0tCj4gVG8gdW5zdWJz Y3JpYmUgZnJvbSB0aGlzIGxpc3Q6IHNlbmQgdGhlIGxpbmUgInVuc3Vic2NyaWJlIGxpbnV4LXdp cmVsZXNzIiBpbgo+IHRoZSBib2R5IG9mIGEgbWVzc2FnZSB0byBtYWpvcmRvbW8tdTc5dXdYTDI5 VFk3Nloyck01bUhYQUBwdWJsaWMuZ21hbmUub3JnCj4gTW9yZSBtYWpvcmRvbW8gaW5mbyBhdCAg aHR0cDovL3ZnZXIua2VybmVsLm9yZy9tYWpvcmRvbW8taW5mby5odG1sCj4gCgpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZnggbWFpbGluZyBs aXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNr dG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types To: David Howells References: <1442842450-29769-1-git-send-email-a.hajda@samsung.com> <17571.1442842945@warthog.procyon.org.uk> Cc: Andrzej Hajda , Bartlomiej Zolnierkiewicz , Marek Szyprowski , linux-kernel@vger.kernel.org, brcm80211-dev-list@broadcom.com, devel@driverdev.osuosl.org, dev@openvswitch.org, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, linux-api@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-cachefs@redhat.com, linux-clk@vger.kernel.org, linux-crypto@vger.kernel.org, linux-fbdev@vger.kernel.org, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, linux-media@vger.kernel.org, linux-mips@linux-mips.org, linux-mm@kvack.org, linux-omap@vger.kernel.org, linux-rdma@vger.kernel.org, linux-serial@vger.kernel.org, linux-sh@vger.kernel.org, linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org, lustre-devel@lists.lustre.org, netdev@vger.kernel.org, rtc-linux@googlegroups.com From: Andrzej Hajda Message-id: <56011BB9.5030004@samsung.com> Date: Tue, 22 Sep 2015 11:13:29 +0200 MIME-version: 1.0 In-reply-to: <17571.1442842945@warthog.procyon.org.uk> Content-type: text/plain; charset=windows-1252 Sender: linux-kernel-owner@vger.kernel.org List-ID: On 09/21/2015 03:42 PM, David Howells wrote: > Andrzej Hajda wrote: > >> Semantic patch finds comparisons of types: >> unsigned < 0 >> unsigned >= 0 >> The former is always false, the latter is always true. >> Such comparisons are useless, so theoretically they could be >> safely removed, but their presence quite often indicates bugs. > > Or someone has left them in because they don't matter and there's the > possibility that the type being tested might be or become signed under some > circumstances. If the comparison is useless, I'd expect the compiler to just > discard it - for such cases your patch is pointless. > > If I have, for example: > > unsigned x; > > if (x == 0 || x > 27) > give_a_range_error(); > > I will write this as: > > unsigned x; > > if (x <= 0 || x > 27) > give_a_range_error(); > > because it that gives a way to handle x being changed to signed at some point > in the future for no cost. In which case, your changing the <= to an == > "because the < part of the case is useless" is arguably wrong. This is why I have not checked for such cases - I have skipped checks of type unsigned <= 0 exactly for the reasons above. However I have left two other checks as they seems to me more suspicious - they are always true or false. But as Dmitry and Andrew pointed out Linus have quite strong opinion against removing range checks in such cases as he finds it clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37. Regards Andrzej > > David > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Date: Tue, 22 Sep 2015 09:13:29 +0000 Subject: Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types Message-Id: <56011BB9.5030004@samsung.com> List-Id: References: <1442842450-29769-1-git-send-email-a.hajda@samsung.com> <17571.1442842945@warthog.procyon.org.uk> In-Reply-To: <17571.1442842945@warthog.procyon.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: David Howells Cc: linux-mips@linux-mips.org, linux-fbdev@vger.kernel.org, linux-sh@vger.kernel.org, brcm80211-dev-list@broadcom.com, dri-devel@lists.freedesktop.org, linux-mm@kvack.org, linux-clk@vger.kernel.org, linux-leds@vger.kernel.org, Marek Szyprowski , devel@driverdev.osuosl.org, linux-rdma@vger.kernel.org, linux-cachefs@redhat.com, linux-serial@vger.kernel.org, linux-input@vger.kernel.org, linux-media@vger.kernel.org, dev@openvswitch.org, rtc-linux@googlegroups.com, Bartlomiej Zolnierkiewicz , intel-gfx@lists.freedesktop.org, linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Andrzej Hajda , linux-api@vger.kernel.org, linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org, netdev@vger.kernel.org, lustre-devel@lists.lustre.org On 09/21/2015 03:42 PM, David Howells wrote: > Andrzej Hajda wrote: > >> Semantic patch finds comparisons of types: >> unsigned < 0 >> unsigned >= 0 >> The former is always false, the latter is always true. >> Such comparisons are useless, so theoretically they could be >> safely removed, but their presence quite often indicates bugs. > > Or someone has left them in because they don't matter and there's the > possibility that the type being tested might be or become signed under some > circumstances. If the comparison is useless, I'd expect the compiler to just > discard it - for such cases your patch is pointless. > > If I have, for example: > > unsigned x; > > if (x = 0 || x > 27) > give_a_range_error(); > > I will write this as: > > unsigned x; > > if (x <= 0 || x > 27) > give_a_range_error(); > > because it that gives a way to handle x being changed to signed at some point > in the future for no cost. In which case, your changing the <= to an = > "because the < part of the case is useless" is arguably wrong. This is why I have not checked for such cases - I have skipped checks of type unsigned <= 0 exactly for the reasons above. However I have left two other checks as they seems to me more suspicious - they are always true or false. But as Dmitry and Andrew pointed out Linus have quite strong opinion against removing range checks in such cases as he finds it clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37. Regards Andrzej > > David > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailout4.w1.samsung.com ([210.118.77.14]:14135 "EHLO mailout4.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by eddie.linux-mips.org with ESMTP id S27007582AbbIVJN7Wa2H2 (ORCPT ); Tue, 22 Sep 2015 11:13:59 +0200 Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NV200D60MZ4DV70@mailout4.w1.samsung.com> for linux-mips@linux-mips.org; Tue, 22 Sep 2015 10:13:52 +0100 (BST) Subject: Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types References: <1442842450-29769-1-git-send-email-a.hajda@samsung.com> <17571.1442842945@warthog.procyon.org.uk> From: Andrzej Hajda Message-ID: <56011BB9.5030004@samsung.com> Date: Tue, 22 Sep 2015 11:13:29 +0200 MIME-version: 1.0 In-reply-to: <17571.1442842945@warthog.procyon.org.uk> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit Return-Path: Sender: linux-mips-bounce@linux-mips.org Errors-to: linux-mips-bounce@linux-mips.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-subscribe: List-owner: List-post: List-archive: To: David Howells Cc: Andrzej Hajda , Bartlomiej Zolnierkiewicz , Marek Szyprowski , linux-kernel@vger.kernel.org, brcm80211-dev-list@broadcom.com, devel@driverdev.osuosl.org, dev@openvswitch.org, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, linux-api@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-cachefs@redhat.com, linux-clk@vger.kernel.org, linux-crypto@vger.kernel.org, linux-fbdev@vger.kernel.org, linux-input@vger.kernel.orglinux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, linux-media@vger.kernel.org, linux-mips@linux-mips.org, linux-mm@kvack.org, linux-omap@vger.kernel.org, linux-rdma@vger.kernel.org, linux-serial@vger.kernel.org, linux-sh@vger.kernel.org, linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org, lustre-devel@lists.lustre.org, netdev@vger.kernel.org, rtc-linux@googlegroups.com Message-ID: <20150922091329.hudXrFDByvQtWsxFfH-vyvklUYxR_BFsXQM4jTCS1j8@z> On 09/21/2015 03:42 PM, David Howells wrote: > Andrzej Hajda wrote: > >> Semantic patch finds comparisons of types: >> unsigned < 0 >> unsigned >= 0 >> The former is always false, the latter is always true. >> Such comparisons are useless, so theoretically they could be >> safely removed, but their presence quite often indicates bugs. > > Or someone has left them in because they don't matter and there's the > possibility that the type being tested might be or become signed under some > circumstances. If the comparison is useless, I'd expect the compiler to just > discard it - for such cases your patch is pointless. > > If I have, for example: > > unsigned x; > > if (x == 0 || x > 27) > give_a_range_error(); > > I will write this as: > > unsigned x; > > if (x <= 0 || x > 27) > give_a_range_error(); > > because it that gives a way to handle x being changed to signed at some point > in the future for no cost. In which case, your changing the <= to an == > "because the < part of the case is useless" is arguably wrong. This is why I have not checked for such cases - I have skipped checks of type unsigned <= 0 exactly for the reasons above. However I have left two other checks as they seems to me more suspicious - they are always true or false. But as Dmitry and Andrew pointed out Linus have quite strong opinion against removing range checks in such cases as he finds it clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37. Regards Andrzej > > David > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout4.w1.samsung.com (mailout4.w1.samsung.com. [210.118.77.14]) by gmr-mx.google.com with ESMTPS id el2si121810pbb.0.2015.09.22.02.13.55 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 22 Sep 2015 02:13:56 -0700 (PDT) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NV200D60MZ4DV70@mailout4.w1.samsung.com> for rtc-linux@googlegroups.com; Tue, 22 Sep 2015 10:13:52 +0100 (BST) Subject: [rtc-linux] Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types To: David Howells References: <1442842450-29769-1-git-send-email-a.hajda@samsung.com> <17571.1442842945@warthog.procyon.org.uk> Cc: Andrzej Hajda , Bartlomiej Zolnierkiewicz , Marek Szyprowski , linux-kernel@vger.kernel.org, brcm80211-dev-list@broadcom.com, devel@driverdev.osuosl.org, dev@openvswitch.org, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, linux-api@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-cachefs@redhat.com, linux-clk@vger.kernel.org, linux-crypto@vger.kernel.org, linux-fbdev@vger.kernel.org, linux-input@vger.kernel.org, linux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, linux-media@vger.kernel.org, linux-mips@linux-mips.org, linux-mm@kvack.org, linux-omap@vger.kernel.org, linux-rdma@vger.kernel.org, linux-serial@vger.kernel.org, linux-sh@vger.kernel.org, linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org, lustre-devel@lists.lustre.org, netdev@vger.kernel.org, rtc-linux@googlegroups.com From: Andrzej Hajda Message-id: <56011BB9.5030004@samsung.com> Date: Tue, 22 Sep 2015 11:13:29 +0200 MIME-version: 1.0 In-reply-to: <17571.1442842945@warthog.procyon.org.uk> Content-type: text/plain; charset=UTF-8 Reply-To: rtc-linux@googlegroups.com List-ID: List-Post: , List-Help: , List-Archive: , List-Unsubscribe: , On 09/21/2015 03:42 PM, David Howells wrote: > Andrzej Hajda wrote: > >> Semantic patch finds comparisons of types: >> unsigned < 0 >> unsigned >= 0 >> The former is always false, the latter is always true. >> Such comparisons are useless, so theoretically they could be >> safely removed, but their presence quite often indicates bugs. > > Or someone has left them in because they don't matter and there's the > possibility that the type being tested might be or become signed under some > circumstances. If the comparison is useless, I'd expect the compiler to just > discard it - for such cases your patch is pointless. > > If I have, for example: > > unsigned x; > > if (x == 0 || x > 27) > give_a_range_error(); > > I will write this as: > > unsigned x; > > if (x <= 0 || x > 27) > give_a_range_error(); > > because it that gives a way to handle x being changed to signed at some point > in the future for no cost. In which case, your changing the <= to an == > "because the < part of the case is useless" is arguably wrong. This is why I have not checked for such cases - I have skipped checks of type unsigned <= 0 exactly for the reasons above. However I have left two other checks as they seems to me more suspicious - they are always true or false. But as Dmitry and Andrew pointed out Linus have quite strong opinion against removing range checks in such cases as he finds it clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37. Regards Andrzej > > David > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- -- You received this message because you are subscribed to "rtc-linux". Membership options at http://groups.google.com/group/rtc-linux . Please read http://groups.google.com/group/rtc-linux/web/checklist before submitting a driver. --- You received this message because you are subscribed to the Google Groups "rtc-linux" group. To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com. For more options, visit https://groups.google.com/d/optout. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrzej Hajda Date: Tue, 22 Sep 2015 11:13:29 +0200 Subject: [lustre-devel] [PATCH 00/38] Fixes related to incorrect usage of unsigned types In-Reply-To: <17571.1442842945@warthog.procyon.org.uk> References: <1442842450-29769-1-git-send-email-a.hajda@samsung.com> <17571.1442842945@warthog.procyon.org.uk> Message-ID: <56011BB9.5030004@samsung.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: lustre-devel@lists.lustre.org On 09/21/2015 03:42 PM, David Howells wrote: > Andrzej Hajda wrote: > >> Semantic patch finds comparisons of types: >> unsigned < 0 >> unsigned >= 0 >> The former is always false, the latter is always true. >> Such comparisons are useless, so theoretically they could be >> safely removed, but their presence quite often indicates bugs. > > Or someone has left them in because they don't matter and there's the > possibility that the type being tested might be or become signed under some > circumstances. If the comparison is useless, I'd expect the compiler to just > discard it - for such cases your patch is pointless. > > If I have, for example: > > unsigned x; > > if (x == 0 || x > 27) > give_a_range_error(); > > I will write this as: > > unsigned x; > > if (x <= 0 || x > 27) > give_a_range_error(); > > because it that gives a way to handle x being changed to signed at some point > in the future for no cost. In which case, your changing the <= to an == > "because the < part of the case is useless" is arguably wrong. This is why I have not checked for such cases - I have skipped checks of type unsigned <= 0 exactly for the reasons above. However I have left two other checks as they seems to me more suspicious - they are always true or false. But as Dmitry and Andrew pointed out Linus have quite strong opinion against removing range checks in such cases as he finds it clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37. Regards Andrzej > > David > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA at public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > From mboxrd@z Thu Jan 1 00:00:00 1970 From: a.hajda@samsung.com (Andrzej Hajda) Date: Tue, 22 Sep 2015 11:13:29 +0200 Subject: [PATCH 00/38] Fixes related to incorrect usage of unsigned types In-Reply-To: <17571.1442842945@warthog.procyon.org.uk> References: <1442842450-29769-1-git-send-email-a.hajda@samsung.com> <17571.1442842945@warthog.procyon.org.uk> Message-ID: <56011BB9.5030004@samsung.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 09/21/2015 03:42 PM, David Howells wrote: > Andrzej Hajda wrote: > >> Semantic patch finds comparisons of types: >> unsigned < 0 >> unsigned >= 0 >> The former is always false, the latter is always true. >> Such comparisons are useless, so theoretically they could be >> safely removed, but their presence quite often indicates bugs. > > Or someone has left them in because they don't matter and there's the > possibility that the type being tested might be or become signed under some > circumstances. If the comparison is useless, I'd expect the compiler to just > discard it - for such cases your patch is pointless. > > If I have, for example: > > unsigned x; > > if (x == 0 || x > 27) > give_a_range_error(); > > I will write this as: > > unsigned x; > > if (x <= 0 || x > 27) > give_a_range_error(); > > because it that gives a way to handle x being changed to signed at some point > in the future for no cost. In which case, your changing the <= to an == > "because the < part of the case is useless" is arguably wrong. This is why I have not checked for such cases - I have skipped checks of type unsigned <= 0 exactly for the reasons above. However I have left two other checks as they seems to me more suspicious - they are always true or false. But as Dmitry and Andrew pointed out Linus have quite strong opinion against removing range checks in such cases as he finds it clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37. Regards Andrzej > > David > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA at public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pa0-f42.google.com (mail-pa0-f42.google.com [209.85.220.42]) by kanga.kvack.org (Postfix) with ESMTP id 3A2166B0038 for ; Tue, 22 Sep 2015 05:13:57 -0400 (EDT) Received: by pacex6 with SMTP id ex6so4250198pac.0 for ; Tue, 22 Sep 2015 02:13:57 -0700 (PDT) Received: from mailout4.w1.samsung.com (mailout4.w1.samsung.com. [210.118.77.14]) by mx.google.com with ESMTPS id r3si1075444pap.0.2015.09.22.02.13.56 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 22 Sep 2015 02:13:56 -0700 (PDT) Received: from eucpsbgm1.samsung.com (unknown [203.254.199.244]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0NV200D60MZ4DV70@mailout4.w1.samsung.com> for linux-mm@kvack.org; Tue, 22 Sep 2015 10:13:52 +0100 (BST) Subject: Re: [PATCH 00/38] Fixes related to incorrect usage of unsigned types References: <1442842450-29769-1-git-send-email-a.hajda@samsung.com> <17571.1442842945@warthog.procyon.org.uk> From: Andrzej Hajda Message-id: <56011BB9.5030004@samsung.com> Date: Tue, 22 Sep 2015 11:13:29 +0200 MIME-version: 1.0 In-reply-to: <17571.1442842945@warthog.procyon.org.uk> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: To: David Howells Cc: Andrzej Hajda , Bartlomiej Zolnierkiewicz , Marek Szyprowski , linux-kernel@vger.kernel.org, brcm80211-dev-list@broadcom.com, devel@driverdev.osuosl.org, dev@openvswitch.org, dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, linux-api@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-cachefs@redhat.com, linux-clk@vger.kernel.org, linux-crypto@vger.kernel.org, linux-fbdev@vger.kernel.org, linux-input@vger.kernel.orglinux-kernel@vger.kernel.org, linux-leds@vger.kernel.org, linux-media@vger.kernel.org, linux-mips@linux-mips.org, linux-mm@kvack.org, linux-omap@vger.kernel.org, linux-rdma@vger.kernel.org, linux-serial@vger.kernel.org, linux-sh@vger.kernel.org, linux-usb@vger.kernel.org, linux-wireless@vger.kernel.org, lustre-devel@lists.lustre.org, netdev@vger.kernel.org, rtc-linux@googlegroups.com On 09/21/2015 03:42 PM, David Howells wrote: > Andrzej Hajda wrote: > >> Semantic patch finds comparisons of types: >> unsigned < 0 >> unsigned >= 0 >> The former is always false, the latter is always true. >> Such comparisons are useless, so theoretically they could be >> safely removed, but their presence quite often indicates bugs. > > Or someone has left them in because they don't matter and there's the > possibility that the type being tested might be or become signed under some > circumstances. If the comparison is useless, I'd expect the compiler to just > discard it - for such cases your patch is pointless. > > If I have, for example: > > unsigned x; > > if (x == 0 || x > 27) > give_a_range_error(); > > I will write this as: > > unsigned x; > > if (x <= 0 || x > 27) > give_a_range_error(); > > because it that gives a way to handle x being changed to signed at some point > in the future for no cost. In which case, your changing the <= to an == > "because the < part of the case is useless" is arguably wrong. This is why I have not checked for such cases - I have skipped checks of type unsigned <= 0 exactly for the reasons above. However I have left two other checks as they seems to me more suspicious - they are always true or false. But as Dmitry and Andrew pointed out Linus have quite strong opinion against removing range checks in such cases as he finds it clearer. I think it applies to patches 29-36. I am not sure about patches 26-28,37. Regards Andrzej > > David > -- > To unsubscribe from this list: send the line "unsubscribe linux-wireless" in > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org