GNOME Bugzilla – Bug 785628
v4l2: Build warning on 32-bit ARM in gstv4l2object.c
Last modified: 2017-08-01 19:09:19 UTC
Created attachment 356640 [details] [review] One possible way to fix the build This is a Yocto build of master branch at 317d3380bbee24763dfeec4bbb9d1c733e519aff (common at 48a5d85ebf4a0bad1c997c83100f710fe2154fbf) | arm-poky-linux-gnueabi-libtool: compile: arm-poky-linux-gnueabi-gcc -march=armv7-a -marm -mfpu=neon -mfloat-abi=hard -mcpu=cortex-a9 --sysroot=/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard -DHAVE_CONFIG_H -I. -I../../../git/sys/v4l2 -I../.. -pthread -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/gstreamer-1.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/glib-2.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/lib/glib-2.0/include -pthread -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/gstreamer-1.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/glib-2.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/lib/glib-2.0/include -I../../../git/gst-libs -pthread -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/gstreamer-1.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/glib-2.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/lib/glib-2.0/include -DGST_USE_UNSTABLE_API -DG_THREADS_MANDATORY -DG_DISABLE_DEPRECATED -Wall -Wdeclaration-after-statement -Wvla -Wpointer-arith -Wmissing-declarations -Wmissing-prototypes -Wredundant-decls -Wwrite-strings -Wold-style-definition -Waggregate-return -Winit-self -Wmissing-include-dirs -Waddress -Wno-multichar -Wnested-externs -Werror -DGST_DISABLE_DEPRECATED -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/gudev-1.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/include/glib-2.0 -I/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard/usr/lib/glib-2.0/include -O2 -pipe -g -feliminate-unused-debug-types -fdebug-prefix-map=/home/user/fsl-community-bsp-2.2/build/tmp/work/cortexa9hf-neon-mx6qdl-poky-linux-gnueabi/gstreamer1.0-plugins-good/1.13+gitAUTOINC+317d3380bb-r0=/usr/src/debug/gstreamer1.0-plugins-good/1.13+gitAUTOINC+317d3380bb-r0 -fdebug-prefix-map=/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/x86_64-linux= -fdebug-prefix-map=/home/user/fsl-community-bsp-2.2/build/tmp/sysroots/wandboard= -c ../../../git/sys/v4l2/gstv4l2object.c -fPIC -DPIC -o .libs/libgstvideo4linux2_la-gstv4l2object.o | ../../../git/sys/v4l2/gstv4l2object.c: In function 'gst_v4l2_object_new': | ../../../git/sys/v4l2/gstv4l2object.c:512:22: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types] | v4l2object->mmap = mmap; | ^ Last argument to mmap is __off_t, which is unsigned long. Last argument to v4l2_mmap is uint64_t. This mismatch breaks build on 32-bit platforms Attached: a patch I used to work around the issue, not sure if this should be ifdef'ed on 32-bit platforms or left to the compiler to eliminate. Alternative patch could use mmap2 to allow for more addressable space if necessary. Assumed regression after commit 31d8a1d929fecd7ceaa7575be5921f9148ed8afb
FYI - if ignored, this causes runtime failures on ARM probably due to differences in 32-bit and 64-bit parameter alignment - you would think that length is just truncated to its 32 least significant bits (on little endian platforms), but instead whatever garbage word is on the stack is passed as 'length' parameter and 64-bit 'length' follows that. I know because I tried adding a cast to '(void*)' in the 'v4l2object->mmap = mmap' assignment and that didn't work as well as I expected ;)
Yes, I know. There is no mmap2 here, but I can see there is a mmap64 which is the same on 64bit platform. Though, this is not in the man-page I got here, I need to figure-out if it's GLIBC specific or if we can find it on other libc. Otherwise we'll just create a wrapper like you proposed on the ML. Will you provide a patch ?
Created attachment 356657 [details] [review] v4l2object: Use mmap64 to match libv4l2 signature I'm not completly certain this is the right fix, since I don't know if this is portable across different libc and OS (notably BSD). It's definatly better then downcasting (hence replacing previous patch). The problem with the blind downcast is that it could break randomly if you have more then 4G of RAM.
Attachment 356657 [details] pushed as b61bba4 - v4l2object: Use mmap64 to match libv4l2 signature
Ok, let 's give this a try. On Linux we need _GNU_SOURCE to be defined, and on BSD, if I read the header properly, mmap64 is always there. It' s also implemented in uClibc, and v4l2 is not built on Android / bionic, since we don 't have permissions to access the define from an Android app anyway.