raspberrypi / utils

A collection of scripts and simple applications
BSD 3-Clause "New" or "Revised" License
134 stars 41 forks source link

vcgencmd: Avoid compiler complaints #65

Open popcornmix opened 5 months ago

popcornmix commented 5 months ago

Latest compilers are more fussy and this doesn't build with debian build scripts. Fix this.

popcornmix commented 5 months ago

@XECDesign does this fix your build issues?

pelwell commented 5 months ago

See #64.

popcornmix commented 5 months ago

Okay - dropped eeptools commit.

XECDesign commented 5 months ago

Seems to build, yeah.

XECDesign commented 5 months ago

I can update the package once this is merged.

popcornmix commented 5 months ago

@pelwell is this okay?

pelwell commented 5 months ago

I don't understand (without seeing the errors) why memset (which will happily copy beyond the end of the string) is somehow more acceptable than strncpy, but I'm happy for this to be merged.

XECDesign commented 5 months ago

Full disclosure: I didn't try building from head of tree without this commit. So if there's a chance it's not needed, I can try without it.

Update: seems happy without it.

XECDesign commented 5 months ago

I was curious why it wasn't failing and it looks like some cmake files have this line: set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wextra -Werror -pedantic")

vcgencmd doesn't, so it only inherits the CFLAGS passed down by Debian's build system - CFLAGS=-g -O2 -ffile-prefix-map=<snip>=. -fstack-protector-strong -Wformat -Werror=format-security

popcornmix commented 5 months ago

I don't understand (without seeing the errors) why memset (which will happily copy beyond the end of the string) is somehow more acceptable than strncpy, but I'm happy for this to be merged.

In the commit message was the error:

In file included from /usr/include/string.h:535, from /<>/vcgencmd/vcgencmd.c:34: In function ‘strncat’, inlined from ‘gencmd’ at /<>/vcgencmd/vcgencmd.c:109:4, inlined from ‘main’ at /<>/vcgencmd/vcgencmd.c:152:14: /usr/include/arm-linux-gnueabihf/bits/string_fortified.h:138:10: warning: ‘builtin___strncat_chk’ specified bound 1024 equals destination size [-Wstringop-overflow=] 138 | return builtin_strncat_chk (dest, src, len, | ^~~~~~~~~~ 139 | __glibc_objsize (__dest)); | ~~~~~

it seems strncat triggers some additional bounds checking that memcpy does not.

chewi commented 5 months ago

Please can we not include -Werror in the default flags as it's a pain for both distributions and users building themselves. Perhaps we could only add it when CMAKE_BUILD_TYPE is Debug.