bluekitchen / btstack

Dual-mode Bluetooth stack, with small memory footprint.
http://bluekitchen-gmbh.com
Other
1.74k stars 618 forks source link

Fix gcc compile warnings #565

Closed ua1arn closed 9 months ago

ua1arn commented 9 months ago

Fix gcc compile warnings (uninitialized variable usage. out-of bound access) with use -Ofast -flto

mringwal commented 9 months ago

Thanks for your contribution. In HFP, there's already an assert which should avoid the warning. I've added additional checks to exit the function if the number of indicators/codecs is too large instead of storing a sub-set (which will make it harder to find the issue).

What's the reason for the change to the hid parser?

ua1arn commented 9 months ago

The reason only for avoid gcc warnings. I see asserts...

ua1arn commented 9 months ago

Unusual way if wrong input data for parser, returned parsed block leaved uninitialized. Noisly output then btstack compilation.

image

mringwal commented 9 months ago

Good catch. I'll fix this later, too.

ua1arn commented 9 months ago

check please comparision indicators and codecs counters in commit 8cd06fb if (hf_indicators_nr > HFP_MAX_NUM_CODECS) return;

ua1arn commented 9 months ago

How about hid_parser?

hfp_hf.c still generate noise: image

gcc options here: arm-none-eabi-gcc -o obj/lib/hftrx_btstack/src/classic/hfp_hf.o -std=gnu99 -Wstrict-prototypes -c -mcpu=cortex-a7 -mfloat-abi=hard -mfpu=neon-vfpv4 -fno-math-errno -funroll-loops -fgraphite-identity -ffunction-sections -fdata-sections -ffat-lto-objects -ftree-vectorize -Ofast -gdwarf-2 -fomit-frame-pointer -Wall -D"NDEBUG"=1 -D"CPUSTYLE_T113"=1 -MD -MP -MF ./obj/lib/hftrx_btstack/src/classic/hfp_hf.o.d -I../../arch/t113s3 -I../../src/hal -I../../lib/dsp3D -I../../CMSIS_5/CMSIS/Core_A/Include -I../../CMSIS_5/CMSIS/Core_A/Source -I../../CMSIS_5/CMSIS/RTOS2/Include -I../../CMSIS_5/CMSIS/RTOS2/RTX/Include -I../../CMSIS-DSP/Include -I../../CMSIS-DSP/ComputeLibrary/Include -I../../CMSIS-DSP/Source -I../../CMSIS-DSP/PrivateInclude -I../../lib/Middlewares/ST/STM32_USB_Device_Library/Core/Inc -I../../lib/hftrx_tinyusb/src -I../.. -I../../inc -I../../lib/hftrx_btstack/src/ble -I../../lib/hftrx_btstack/src/ble/gatt-service -I../../lib/hftrx_btstack/src/classic -I../../lib/hftrx_btstack/src -I../../lib/hftrx_btstack/3rd-party/micro-ecc -I../../lib/hftrx_btstack/3rd-party/bluedroid/decoder/include -I../../lib/hftrx_btstack/3rd-party/bluedroid/encoder/include -I../../lib/hftrx_btstack/3rd-party/hxcmod-player -I../../lib/hftrx_btstack/3rd-party/hxcmod-player/mods -I../../lib/hftrx_btstack/3rd-party/lwip/core/src/include -I../../lib/hftrx_btstack/3rd-party/lwip/dhcp-server -I../../lib/hftrx_btstack/3rd-party/md5 -I../../lib/hftrx_btstack/3rd-party/yxml -I../../lib/hftrx_btstack/3rd-party/segger-rtt -I../../lib/hftrx_btstack/platform/embedded -I../../lib/hftrx_btstack/platform/lwip -I../../lib/hftrx_btstack/platform/lwip/port ../../lib/hftrx_btstack/src/classic/hfp_hf.c

mringwal commented 9 months ago

check please comparision indicators and codecs counters in commit 8cd06fb if (hf_indicators_nr > HFP_MAX_NUM_CODECS) return;

Thanks. That should be HFP_MAX_NUM_INDICATORS

mringwal commented 9 months ago

The warning in hfp_ag_init_hf_indicators might be caused by the incorrect comparison to HFP_MAX_NUM_CODECS instead of HFP_MAX_NUM_INDICATORS. Please update to develop and check again. From reading the code, it should be correct now.

I'll look at the hid parser next week. We need to discuss how to propagate hid parsing errors properly to the caller.

mringwal commented 9 months ago

We have reworked the hid parser on develop and return parsing state for btstack_hid_parse_descriptor_item plus handle error in all calling functions, which should fix the valid compile warnings.