Hi,
This is set of independent patches. The only connection between
them is that they try to address problems spotted by proposed
coccinelle semantic patch unsigned_lesser_than_zero.cocci[1].
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.
This patchset contains mainly real bug fixes(patches 01-25),
usually type fixes.
Patches 26-37 removes unnecessary checks. Semantic patch found
much more such places (more than 80), but since every case needs
some analysis I have decided to leave them for antoher patchset.
The last patch should be probably replaced with something better,
I guess it should be treated rather as bug report.
The patches are based on linux-next (20150918).
Patches were only compile tested, so please look at them carefully.
I have sent all patches to linux-kernel mailing list. Individual
patches + cover letter went also to apropriate addresses,
according to get_maintainers.pl script.
One more thing. To fullfill different maintaner/subsystem requirements
I have decided to prefix patch subjects with prefixes present in
last 10 commits for affected files. I am not sure if this is the
best solution, if there are any better solutions please let me know :)
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Regards
Andrzej
Andrzej Hajda (38):
arm-cci: fix handling cpumask_any_but return value
bus: arm-ccn: fix handling cpumask_any_but return value
drm/i915: fix handling gen8_emit_flush_coherentl3_wa result
IB/ehca: fix handling idr_alloc result
staging: lustre: fix handling lustre_posix_acl_xattr_filter result
tty: serial: lpc32xx_hs: fix handling platform_get_irq result
usb: host: ehci-msm: fix handling platform_get_irq result
openvswitch: fix handling result of ipv6_skip_exthdr
selftests/timers: fix write return value handlng
hwrng: fix handling platform_get_irq
HSI: omap_ssi: fix handling ida_simple_get result
HSI: omap_ssi_port: fix handling of_get_named_gpio result
ARM: shmobile: apmu: correct type of CPU id
clk: vt8500: fix sign of possible PLL values
drm/layerscape: fix handling fsl_dcu_drm_plane_index result
gpu: ipu-v3: fix div_ratio type
isdn: hisax: fix frame calculation
net/ibm/emac: fix type of phy_mode
net: stmmac: fix type of entry variable
net: brcm80211: fix range check
mwifiex: fix comparison expression
orinoco: fix checking for default value
rndis_wlan: fix checking for default value
rtc: opal: fix type of token
staging: media: davinci_vpfe: fix ipipe_mode type
staging: lustre: remove invalid check
usbnet: remove invalid check
video/omap: remove invalid check
Input: touchscreen: atmel: remove invalid check
leds: flash: remove invalid check
leds: tca6507: remove invalid check
fs/cachefiles: remove invalid checks
mm/memblock.c: remove invalid check
perf: remove invalid check
ptrace: remove invalid check
MIPS: remove invalid check
zlib_deflate/deftree: change always true condition to 1
drm/radeon: simplify boot level calculation
arch/arm/mach-shmobile/platsmp-apmu.c | 2 +-
arch/mips/mm/sc-mips.c | 4 ++--
arch/sh/kernel/ptrace_32.c | 3 +--
arch/sh/kernel/ptrace_64.c | 4 ++--
drivers/bus/arm-cci.c | 2 +-
drivers/bus/arm-ccn.c | 2 +-
drivers/char/hw_random/xgene-rng.c | 7 ++++---
drivers/clk/clk-vt8500.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/kv_dpm.c | 11 +----------
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 3 ++-
drivers/gpu/drm/i915/intel_lrc.c | 7 ++++---
drivers/gpu/drm/radeon/kv_dpm.c | 11 +----------
drivers/gpu/ipu-v3/ipu-csi.c | 2 +-
drivers/hsi/controllers/omap_ssi.c | 7 +++----
drivers/hsi/controllers/omap_ssi_port.c | 8 ++++----
drivers/input/touchscreen/atmel_mxt_ts.c | 2 +-
drivers/isdn/hisax/hfc4s8s_l1.c | 10 +++++-----
drivers/leds/led-class-flash.c | 2 +-
drivers/leds/leds-tca6507.c | 2 +-
drivers/net/ethernet/ibm/emac/core.h | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
drivers/net/usb/lan78xx.c | 5 -----
drivers/net/usb/smsc75xx.c | 5 -----
drivers/net/usb/smsc95xx.c | 5 -----
drivers/net/wireless/brcm80211/brcmsmac/main.c | 2 +-
drivers/net/wireless/mwifiex/11n_rxreorder.c | 4 ++--
drivers/net/wireless/orinoco/cfg.c | 6 +++---
drivers/net/wireless/rndis_wlan.c | 2 +-
drivers/rtc/rtc-opal.c | 4 ++--
drivers/staging/lustre/lustre/llite/xattr.c | 7 ++++---
drivers/staging/lustre/lustre/osc/lproc_osc.c | 3 ---
drivers/staging/media/davinci_vpfe/dm365_ipipe_hw.c | 2 +-
drivers/staging/rdma/ehca/ehca_cq.c | 13 +++++++------
drivers/tty/serial/lpc32xx_hs.c | 7 ++++---
drivers/usb/host/ehci-msm.c | 6 +++---
drivers/video/fbdev/omap/omapfb_main.c | 5 -----
fs/cachefiles/bind.c | 9 +++------
fs/cachefiles/daemon.c | 6 +++---
lib/zlib_deflate/deftree.c | 2 +-
mm/memblock.c | 2 +-
net/openvswitch/conntrack.c | 2 +-
tools/testing/selftests/timers/clocksource-switch.c | 2 +-
42 files changed, 79 insertions(+), 119 deletions(-)
--
1.9.1
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Andrzej Hajda <[email protected]> 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.
David
The function can return negative value.
The problem has been detected using proposed semantic patch
scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
[1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
Signed-off-by: Andrzej Hajda <[email protected]>
---
drivers/char/hw_random/xgene-rng.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/char/hw_random/xgene-rng.c b/drivers/char/hw_random/xgene-rng.c
index c37cf75..3c77645 100644
--- a/drivers/char/hw_random/xgene-rng.c
+++ b/drivers/char/hw_random/xgene-rng.c
@@ -344,11 +344,12 @@ static int xgene_rng_probe(struct platform_device *pdev)
if (IS_ERR(ctx->csr_base))
return PTR_ERR(ctx->csr_base);
- ctx->irq = platform_get_irq(pdev, 0);
- if (ctx->irq < 0) {
+ rc = platform_get_irq(pdev, 0);
+ if (rc < 0) {
dev_err(&pdev->dev, "No IRQ resource\n");
- return ctx->irq;
+ return rc;
}
+ ctx->irq = rc;
dev_dbg(&pdev->dev, "APM X-Gene RNG BASE %p ALARM IRQ %d",
ctx->csr_base, ctx->irq);
--
1.9.1
On Mon, Sep 21, 2015 at 03:33:42PM +0200, Andrzej Hajda wrote:
> The function can return negative value.
>
> The problem has been detected using proposed semantic patch
> scripts/coccinelle/tests/unsigned_lesser_than_zero.cocci [1].
>
> [1]: http://permalink.gmane.org/gmane.linux.kernel/2038576
>
> Signed-off-by: Andrzej Hajda <[email protected]>
Applied to crypto.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 09/21/2015 03:42 PM, David Howells wrote:
> Andrzej Hajda <[email protected]> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On 09/22/2015 11:13 AM, Andrzej Hajda wrote:
> On 09/21/2015 03:42 PM, David Howells wrote:
>> Andrzej Hajda <[email protected]> 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.
Dropped 30/38 and 31/38 from LED tree then.
--
Best Regards,
Jacek Anaszewski
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>