From: Wen Gong <wgong@codeaurora.org>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: kbuild-all@lists.01.org, linux-kernel@vger.kernel.org,
ath10k@lists.infradead.org,
kernel test robot <rong.a.chen@intel.com>
Subject: Re: drivers/net/wireless/ath/ath10k/sdio.c:2234:2: warning: Non-boolean value returned from function returning bool
Date: Mon, 16 Nov 2020 17:33:34 +0800 [thread overview]
Message-ID: <32eb018069a079fd82e359d0ed4e1b1e@codeaurora.org> (raw)
In-Reply-To: <87o8jx20w6.fsf@codeaurora.org>
On 2020-11-16 15:53, Kalle Valo wrote:
> + ath10k list
>
> kernel test robot <rong.a.chen@intel.com> writes:
>
>> tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> master
>> head: f01c30de86f1047e9bae1b1b1417b0ce8dcd15b1
>> commit: 3c45f21af84eb05a355919abc80cf70a3a681cee ath10k: sdio: add
>> firmware coredump support
>> compiler: nios2-linux-gcc (GCC) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <rong.a.chen@intel.com>
>>
>>
>> cppcheck possible warnings: (new ones prefixed by >>, may not real
>> problems)
>>
>>>> drivers/net/wireless/ath/ath10k/sdio.c:2234:2: warning: Non-boolean
>> value returned from function returning bool
>> [returnNonBoolInBooleanFunction]
>> return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
>> ^
>
> Is this really a problem? I guess we could change that to "!!(param &
> HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW)" but how is that better and
> does it make any practical difference when
> ath10k_sdio_is_fast_dump_supported() returns a boolean anyway?
It should not be a real problem by test before.
ath10k_sdio_is_fast_dump_supported return true in test before.
and fast_dump get true from ath10k_sdio_is_fast_dump_supported in
ath10k_sdio_fw_crashed_dump.
the risk maybe like this:
u32 param in ath10k_sdio_is_fast_dump_supported is 4 bytes, and bool is
1 byte.
if Gcc does not do any convertion from u32 to bool, then the bool value
will only
get the lower byte(byte 0) of the 4 bytes, then maybe get an mistake
result.
I did a disassemble of sdio.o with/without below change, and dump
sdio.o, it have no difference in this place:
I change send a patch with below change if needed.
@@ -2231,7 +2231,7 @@ static bool
ath10k_sdio_is_fast_dump_supported(struct ath10k *ar)
ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio hi_option_flag2 %x\n",
param);
- return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
+ return !!(param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW);
}
objdump -Sl sdio.o > sdio.o-Sl.txt
0000000000002b00 <ath10k_sdio_fw_crashed_dump>:
void ath10k_sdio_fw_crashed_dump(struct ath10k *ar)
{
...
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2234
return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
2b75: 8b 44 24 44 mov 0x44(%rsp),%eax
2b79: 25 00 08 00 00 and $0x800,%eax //"the SF, ZF, and PF
flags are set according to the result".ZF will be 0 if true.
2b7e: 89 44 24 2c mov %eax,0x2c(%rsp)
2b82: 40 0f 95 c5 setne %bpl //"Set byte if not equal
(ZF=0)".stored the ZF flag to bpl as value of fast_dump.
ath10k_sdio_fw_crashed_dump():
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2483
char guid[UUID_STRING_LEN + 1];
bool fast_dump;
fast_dump = ath10k_sdio_is_fast_dump_supported(ar);
if (fast_dump)
2b86: 0f 85 c3 03 00 00 jne 2f4f
<ath10k_sdio_fw_crashed_dump+0x44f> //"Jump near if not equal (ZF=0)"
...
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2499
ath10k_sdio_dump_registers(ar, crash_data, fast_dump);
2be5: 40 0f b6 d5 movzbl %bpl,%edx //use the bpl stored as
value of fast_dump
2be9: 48 89 de mov %rbx,%rsi
2bec: 4c 89 ff mov %r15,%rdi
2bef: e8 8c fb ff ff callq 2780 <ath10k_sdio_dump_registers>
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
WARNING: multiple messages have this Message-ID (diff)
From: Wen Gong <wgong@codeaurora.org>
To: kbuild-all@lists.01.org
Subject: Re: drivers/net/wireless/ath/ath10k/sdio.c:2234:2: warning: Non-boolean value returned from function returning bool
Date: Mon, 16 Nov 2020 17:33:34 +0800 [thread overview]
Message-ID: <32eb018069a079fd82e359d0ed4e1b1e@codeaurora.org> (raw)
In-Reply-To: <87o8jx20w6.fsf@codeaurora.org>
[-- Attachment #1: Type: text/plain, Size: 3552 bytes --]
On 2020-11-16 15:53, Kalle Valo wrote:
> + ath10k list
>
> kernel test robot <rong.a.chen@intel.com> writes:
>
>> tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> master
>> head: f01c30de86f1047e9bae1b1b1417b0ce8dcd15b1
>> commit: 3c45f21af84eb05a355919abc80cf70a3a681cee ath10k: sdio: add
>> firmware coredump support
>> compiler: nios2-linux-gcc (GCC) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <rong.a.chen@intel.com>
>>
>>
>> cppcheck possible warnings: (new ones prefixed by >>, may not real
>> problems)
>>
>>>> drivers/net/wireless/ath/ath10k/sdio.c:2234:2: warning: Non-boolean
>> value returned from function returning bool
>> [returnNonBoolInBooleanFunction]
>> return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
>> ^
>
> Is this really a problem? I guess we could change that to "!!(param &
> HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW)" but how is that better and
> does it make any practical difference when
> ath10k_sdio_is_fast_dump_supported() returns a boolean anyway?
It should not be a real problem by test before.
ath10k_sdio_is_fast_dump_supported return true in test before.
and fast_dump get true from ath10k_sdio_is_fast_dump_supported in
ath10k_sdio_fw_crashed_dump.
the risk maybe like this:
u32 param in ath10k_sdio_is_fast_dump_supported is 4 bytes, and bool is
1 byte.
if Gcc does not do any convertion from u32 to bool, then the bool value
will only
get the lower byte(byte 0) of the 4 bytes, then maybe get an mistake
result.
I did a disassemble of sdio.o with/without below change, and dump
sdio.o, it have no difference in this place:
I change send a patch with below change if needed.
@@ -2231,7 +2231,7 @@ static bool
ath10k_sdio_is_fast_dump_supported(struct ath10k *ar)
ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio hi_option_flag2 %x\n",
param);
- return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
+ return !!(param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW);
}
objdump -Sl sdio.o > sdio.o-Sl.txt
0000000000002b00 <ath10k_sdio_fw_crashed_dump>:
void ath10k_sdio_fw_crashed_dump(struct ath10k *ar)
{
...
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2234
return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
2b75: 8b 44 24 44 mov 0x44(%rsp),%eax
2b79: 25 00 08 00 00 and $0x800,%eax //"the SF, ZF, and PF
flags are set according to the result".ZF will be 0 if true.
2b7e: 89 44 24 2c mov %eax,0x2c(%rsp)
2b82: 40 0f 95 c5 setne %bpl //"Set byte if not equal
(ZF=0)".stored the ZF flag to bpl as value of fast_dump.
ath10k_sdio_fw_crashed_dump():
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2483
char guid[UUID_STRING_LEN + 1];
bool fast_dump;
fast_dump = ath10k_sdio_is_fast_dump_supported(ar);
if (fast_dump)
2b86: 0f 85 c3 03 00 00 jne 2f4f
<ath10k_sdio_fw_crashed_dump+0x44f> //"Jump near if not equal (ZF=0)"
...
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2499
ath10k_sdio_dump_registers(ar, crash_data, fast_dump);
2be5: 40 0f b6 d5 movzbl %bpl,%edx //use the bpl stored as
value of fast_dump
2be9: 48 89 de mov %rbx,%rsi
2bec: 4c 89 ff mov %r15,%rdi
2bef: e8 8c fb ff ff callq 2780 <ath10k_sdio_dump_registers>
WARNING: multiple messages have this Message-ID (diff)
From: Wen Gong <wgong@codeaurora.org>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: kernel test robot <rong.a.chen@intel.com>,
kbuild-all@lists.01.org, linux-kernel@vger.kernel.org,
ath10k@lists.infradead.org
Subject: Re: drivers/net/wireless/ath/ath10k/sdio.c:2234:2: warning: Non-boolean value returned from function returning bool
Date: Mon, 16 Nov 2020 17:33:34 +0800 [thread overview]
Message-ID: <32eb018069a079fd82e359d0ed4e1b1e@codeaurora.org> (raw)
In-Reply-To: <87o8jx20w6.fsf@codeaurora.org>
On 2020-11-16 15:53, Kalle Valo wrote:
> + ath10k list
>
> kernel test robot <rong.a.chen@intel.com> writes:
>
>> tree:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>> master
>> head: f01c30de86f1047e9bae1b1b1417b0ce8dcd15b1
>> commit: 3c45f21af84eb05a355919abc80cf70a3a681cee ath10k: sdio: add
>> firmware coredump support
>> compiler: nios2-linux-gcc (GCC) 9.3.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <rong.a.chen@intel.com>
>>
>>
>> cppcheck possible warnings: (new ones prefixed by >>, may not real
>> problems)
>>
>>>> drivers/net/wireless/ath/ath10k/sdio.c:2234:2: warning: Non-boolean
>> value returned from function returning bool
>> [returnNonBoolInBooleanFunction]
>> return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
>> ^
>
> Is this really a problem? I guess we could change that to "!!(param &
> HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW)" but how is that better and
> does it make any practical difference when
> ath10k_sdio_is_fast_dump_supported() returns a boolean anyway?
It should not be a real problem by test before.
ath10k_sdio_is_fast_dump_supported return true in test before.
and fast_dump get true from ath10k_sdio_is_fast_dump_supported in
ath10k_sdio_fw_crashed_dump.
the risk maybe like this:
u32 param in ath10k_sdio_is_fast_dump_supported is 4 bytes, and bool is
1 byte.
if Gcc does not do any convertion from u32 to bool, then the bool value
will only
get the lower byte(byte 0) of the 4 bytes, then maybe get an mistake
result.
I did a disassemble of sdio.o with/without below change, and dump
sdio.o, it have no difference in this place:
I change send a patch with below change if needed.
@@ -2231,7 +2231,7 @@ static bool
ath10k_sdio_is_fast_dump_supported(struct ath10k *ar)
ath10k_dbg(ar, ATH10K_DBG_SDIO, "sdio hi_option_flag2 %x\n",
param);
- return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
+ return !!(param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW);
}
objdump -Sl sdio.o > sdio.o-Sl.txt
0000000000002b00 <ath10k_sdio_fw_crashed_dump>:
void ath10k_sdio_fw_crashed_dump(struct ath10k *ar)
{
...
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2234
return param & HI_OPTION_SDIO_CRASH_DUMP_ENHANCEMENT_FW;
2b75: 8b 44 24 44 mov 0x44(%rsp),%eax
2b79: 25 00 08 00 00 and $0x800,%eax //"the SF, ZF, and PF
flags are set according to the result".ZF will be 0 if true.
2b7e: 89 44 24 2c mov %eax,0x2c(%rsp)
2b82: 40 0f 95 c5 setne %bpl //"Set byte if not equal
(ZF=0)".stored the ZF flag to bpl as value of fast_dump.
ath10k_sdio_fw_crashed_dump():
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2483
char guid[UUID_STRING_LEN + 1];
bool fast_dump;
fast_dump = ath10k_sdio_is_fast_dump_supported(ar);
if (fast_dump)
2b86: 0f 85 c3 03 00 00 jne 2f4f
<ath10k_sdio_fw_crashed_dump+0x44f> //"Jump near if not equal (ZF=0)"
...
/home/wgong/ath11k/ath-master/ath/drivers/net/wireless/ath/ath10k/sdio.c:2499
ath10k_sdio_dump_registers(ar, crash_data, fast_dump);
2be5: 40 0f b6 d5 movzbl %bpl,%edx //use the bpl stored as
value of fast_dump
2be9: 48 89 de mov %rbx,%rsi
2bec: 4c 89 ff mov %r15,%rdi
2bef: e8 8c fb ff ff callq 2780 <ath10k_sdio_dump_registers>
next prev parent reply other threads:[~2020-11-16 9:35 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-16 5:53 drivers/net/wireless/ath/ath10k/sdio.c:2234:2: warning: Non-boolean value returned from function returning bool kernel test robot
2020-11-16 5:53 ` kernel test robot
2020-11-16 7:53 ` Kalle Valo
2020-11-16 7:53 ` Kalle Valo
2020-11-16 7:53 ` Kalle Valo
2020-11-16 9:33 ` Wen Gong [this message]
2020-11-16 9:33 ` Wen Gong
2020-11-16 9:33 ` Wen Gong
-- strict thread matches above, loose matches on Subject: below --
2020-11-14 18:33 kernel test robot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=32eb018069a079fd82e359d0ed4e1b1e@codeaurora.org \
--to=wgong@codeaurora.org \
--cc=ath10k@lists.infradead.org \
--cc=kbuild-all@lists.01.org \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rong.a.chen@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.