Possible bug in linux-6.2/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c
Hello there, I ran the static analyser cppcheck over the linux-6.2 source code and got this: linux-6.2/tools/testing/selftests/powerpc/pmu/sampling_tests/mmcra_thresh_marked_sample_test.c:68:10: style: Same expression '0x3' found multiple times in chain of '&' operators. [duplicateExpression] Source code is FAIL_IF(EV_CODE_EXTRACT(event.attr.config, sample & 0x3) != get_mmcra_sample_mode(get_reg_value(intr_regs, "MMCRA"), 4)); but #define EV_CODE_EXTRACT(x, y) \ ((x >> ev_shift_##y) & ev_mask_##y) Given the token pasting, I very much doubt an expression like "sample & 0x3" will work correctly. Same thing on the line above FAIL_IF(EV_CODE_EXTRACT(event.attr.config, sample >> 2) != get_mmcra_rand_samp_elig(get_reg_value(intr_regs, "MMCRA"), 4)); "sample >> 2" doesn't look like a valid token to me. Regards David Binderman
linux-4.16-rc1/drivers/misc/ocxl/file.c:320:broken error checking ?
Hello there, linux-4.16-rc1/drivers/misc/ocxl/file.c:320]: (style) Checking if unsigned variable 'used' is less than zero. Source code is used = append_xsl_error(ctx, , buf + sizeof(header)); if (used < 0) return used; Suggest put return value from function into signed variable, sanity check it, then assign it to an unsigned variable. Also, use of the gcc compiler flag -Wtype-limits will show up this kind of problem in future. Regards David Binderman
linux-4.14-rc3/arch/powerpc/perf/imc-pmu.c:599: pointless test ?
Hello there,
linux-4.14-rc3/arch/powerpc/perf/imc-pmu.c:599]: (style) Unsigned variable
'ncpu' can't be negative so it is unnecessary to test it.
Source code is
if (ncpu >= 0 && ncpu < nr_cpu_ids) {
but
unsigned int ncpu, core_id;
Suggest remove test.
Regards
David Binderman
linux-4.14-rc1/arch/powerpc/perf/hv-24x7.c:541: bad condition ?
Hello there, linux-4.14-rc1/arch/powerpc/perf/hv-24x7.c:543]: (warning) Identical condition 's1<s2', second condition is always false Source code is if (s1 < s2) return 1; if (s2 > s1) return -1; Suggest code rework. Regards David Binderman
Re: arch/powerpc/xmon/dis-asm.h: 2 * wrong specifiers ?
Hello there,
On Tue, Jun 28, 2016 at 5:08 AM, Michael Ellerman <m...@ellerman.id.au> wrote:
> What config / toolchain are you using? I've never seen these.
A static analyser for C & C++ called cppcheck. Available from sourceforge.
I think you can also get a similar warning if you tweek the gcc compiler warning
flags. -Wformat=2 maybe.
>> static inline int print_insn_powerpc(unsigned long insn, unsigned long
>> memaddr)
>> {
>> printf("%.8x", insn);
>> return 0;
>> }
>
> Send me a patch to cast insn to unsigned int?
I don't know the code, but given that insn is unsigned long and so can go
past 32 bits, using a cast to unsigned int might throw away the
possibly important
upper bits.
Regards
David Binderman
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
arch/powerpc/xmon/dis-asm.h: 2 * wrong specifiers ?
Hello there,
linux-4.7-rc5/arch/powerpc/xmon/dis-asm.h:20]: (warning) %x in format
string (no. 1) requires 'unsigned int' but the argument type is
'unsigned long'.
[linux-4.7-rc5/arch/powerpc/xmon/dis-asm.h:26]: (warning) %x in format
string (no. 1) requires 'unsigned int' but the argument type is
'unsigned long'.
Source code is
static inline int print_insn_powerpc(unsigned long insn, unsigned long memaddr)
{
printf("%.8x", insn);
return 0;
}
static inline int print_insn_spu(unsigned long insn, unsigned long memaddr)
{
printf("%.8x", insn);
return 0;
}
Regards
David Binderman
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
arch/powerpc/platforms/512x/clock-commonclk.c:824: wrong % specifier ?
Hello there,
linux-4.7-rc5/arch/powerpc/platforms/512x/clock-commonclk.c:824]:
(warning) %d in format string (no. 1) requires 'int' but the argument
type is 'size_t {aka unsigned long}'.
Source code is
snprintf(name, sizeof(name), "psc%d", mclk_idx);
Regards
David Binderman
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
RE: linux-4.5-rc4/arch/powerpc/boot/treeboot-akebono.c:90: possible bad test ?
Hello there Daniel, > That looks like a good suggestion: maybe make the test 'if (!emac)' > rather than explicitly comparing with zero. Righto. > Are you comfortable sending a patch to that effect? Sorry, no. My email provider can't implement the somewhat strict whitespace rules of kernel patches. Nothing stopping any keen person implementing that patch, however ;-> I checked the rest of the treeboot-akebono.c source code file with the static analyser and found nothing wrong. No warnings from gcc -Wextra, either, so after the patch, it looks ok to me. Regards David Binderman ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
linux-4.5-rc4/arch/powerpc/boot/treeboot-akebono.c:90: possible bad test ?
hello there,
[linux-4.5-rc4/arch/powerpc/boot/treeboot-akebono.c:90]: (style) A pointer can
not be negative so it is either pointless or an error to check if it is not.
Source code is
emac = finddevice("/plb/opb/ethernet");
if (emac> 0) {
but
void *emac;
Suggest new code
emac = finddevice("/plb/opb/ethernet");
if (emac != 0) {
Regards
David Binderman
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
arch/powerpc/kernel/cacheinfo.c: Possible null pointer dereference
Hello there, I just tried out cppcheck-1.47 on the linux-2.6.38-rc4 source code. It said [arch/powerpc/kernel/cacheinfo.c:380]: (error) Possible null pointer dereference: cache - otherwise it is redundant to check if cache is null at line 382 The source code is WARN_ONCE(cache cache-level != level, cache level mismatch on lookup (got %d, expected %d)\n, cache-level, level); if (!cache) I agree with cppcheck. Suggest code rework. Regards David Binderman ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
