bluekitchen / btstack

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

(small fix) Use memcpy instead of copying bytes manually #549

Open cadouthat opened 10 months ago

cadouthat commented 10 months ago

This is cleaner and more efficient, but more importantly it fixes this annoying GCC warning:

hfp_hf.c:1561:26: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
 1561 |         hfp_hf_codecs[i] = codecs[i];
      |         ~~~~~~~~~~~~~~~~~^~~~~~~~~~~
mringwal commented 10 months ago

Thanks for the fix/workaround. I'd like to understand this better first.

Which gcc gives you this warning? Also, looking hard at the code, I don't see the problem. There's an global array of HFP_MAX_NUM_CODECS. Do you have btstack_assert enabled?

Could you try for (i=0; i<codecs_nr && i < HFP_MAX_NUM_CODECS; i++){ .. }?

cadouthat commented 10 months ago

I'm using GCC 10.3.1 arm-none-eabi (Pico SDK). Agreed that there doesn't appear to be anything wrong with the code, my guess is that GCC somehow mis-interprets hfp_hf_codecs as a null-terminated string, and believes that we may not be leaving room for the terminator.

btstack_assert does appear to be enabled, and still no luck with && i < HFP_MAX_NUM_CODECS

hegdi commented 10 months ago

Well I'm afraid this is a bug but not on our side, checkout https://gcc.gnu.org/g:b48d4e6818674898f90d9358378c127511ef0f9f for explanation/resolution or https://gcc.gnu.org/bugzilla/show_bug.cgi?id=101854 for a little back story, fixed in GCC 12 and I can confirm GCC 13.2.0 is not affected. Well I would suggest you update your arm-none-eabi toolchain the current Debian is at GCC 12.2.1 for arm, from 2022, so GCC 10.3.1 is pretty old

cadouthat commented 10 months ago

Thanks for the info! Unfortunately I think my version is the most recent via Homebrew, so I'd have to install manually

But regardless of the GCC warning, I still think the proposed change is good in itself

mringwal commented 10 months ago

@cadouthat Interestingly, my manually installed arm-none-eabi toolchain is 10.3-2021.10, while brew provides an up-to-date 13.2.0. I guess I'll switch to the one provided by brew - which is even compile for ARM64 instead of emulating x64 on my M2.