bluetooth: shell: Lots of checks of type (unsigned < 0) which is bogus

[finikorg]

Unsigned cannot be less then zero so there is no need to check for it.

Example of checks:

zephyr/subsys/bluetooth/shell/bap_broadcast_assistant.c

Lines 315 to 316 in cf871ae

	param.adv_sid = strtol(argv[3], NULL, 0);
	if (param.adv_sid < 0 || param.adv_sid > 0x0F) {

or

zephyr/subsys/bluetooth/shell/bap_broadcast_assistant.c

Lines 321 to 322 in cf871ae

	param.pa_sync = strtol(argv[4], NULL, 0);
	if (param.pa_sync < 0 || param.pa_sync > 1) {

etc

[Thalley]

@finikorg Do you have a way to generate the full list?

It's quite an effort if someone manually have to go through each strtoX in the shell modules to verify every single parameter.

[finikorg]

No, I cannot, but I think it does make sense to check them all, there are other bugs like checking (uint32 > UINT32_MAX), etc.
I am sure all tools like Klocwork, coverity, etc can help with that.

[Thalley]

No, I cannot, but I think it does make sense to check them all, there are other bugs like checking (uint32 > UINT32_MAX), etc

You may be right :) Looks like we have a total of 286 calls to strtoX that needs to be checked.

I am sure all tools like Klocwork, coverity, etc can help with that.
Except none of those are free to use :(

There are several discussions going on with using cppcheck or other free tools, but I wonder if they also catch these :)

But yeah, without a predefined list, fixing this issue may take quite a while, as it is low priority but high effort.

I can put in the work to fix this for the audio shell modules.

[Thalley]

Fixing all LE Audio (and ISO) shell strtoX in #54165

[Thalley]

@finikorg I think I fixed all the bugs in the LE audio files. Please review :)

[Thalley]

@finikorg Can you verify if there are still some missing in the non-LE Audio files?

[finikorg]

Can you verify if there are still some missing in the non-LE Audio files?

Thanks. I think all fixed, I let you know if there are some bugs left.