GNOME Bugzilla – Bug 791779
v4l2src: mmap64 is not available on FreeBSD
Last modified: 2018-02-18 21:27:33 UTC
Commit b61bba4 makes sys/v4l2/gstv4l2object.c use mmap64 unconditionally, but mmap64 is not specified in the POSIX standard and it is not available on some systems. We have to check for it before using it to keep gst-plugins-good work on FreeBSD.
Created attachment 365739 [details] [review] v4l2object: Check for mmap64 before using it mmap64 is not available on FreeBSD.
Should mmap64 be used directly? Shouldn't we rather use mmap and set some flags so that it maps to 64-bit sizes?
Review of attachment 365739 [details] [review]: I think for freebsd, We will need a wrapper to check the overflow. Also you are assigning to an incompatible function pointer. Its strange since I thought it was supported by other flavour of bsd, maybe I missed checked. Means you cannot mmap large files on FreeBSD?
(In reply to Tim-Philipp Müller from comment #2) > Should mmap64 be used directly? Shouldn't we rather use mmap and set some > flags so that it maps to 64-bit sizes? The function signature are from libv4l2. We should always prefer 64 bit variant in newly written code imho.
I'm not sure that mmap64 not being part of POSIX is right.
mmap on FreeBSD is declared in sys/mman.h as: void * mmap(void *, size_t, int, int, int, off_t); and off_t is defined in sys/_types.h as int64_t. off_t is not defined in a machine-dependent header file, so I think it is always 64-bit regardless of architecture.
Ok, so FreeBSD is hardcoding it to be equivalent of mmap64 on Linux, regardless of if you are running on 32bit or not. Can you then provide a cleaner patch, something that makes this clear and FreeBSD or better sizeof(off_t) == 8 specific ? Something that ends up doing: #ifdef ... #define mmap64 mmap #endif
Review of attachment 365739 [details] [review]: ::: sys/v4l2/gstv4l2object.c @@ +519,3 @@ v4l2object->mmap = mmap64; +#else + v4l2object->mmap = mmap; To explain better, here you assume that not having mmap64 implies that off_t == int64_t . Check for the exact thing you are looking for, e.g. that sizeof(off_t) == 8. The signature I'm trying to match is the following: LIBV4L_PUBLIC void *v4l2_mmap(void *start, size_t length, int prot, int flags, int fd, int64_t offset);
Ok, both OpenBSD and FreeBSD matches libv4l2 signature of v4l2_mmap. Let's assume if a plaform defines mmap64, then it shall be used. So I agree with your patch.
Review of attachment 365739 [details] [review]: Let's merge this.
Thanks, and sorry for the confusion. Attachment 365739 [details] pushed as 7843482 - v4l2object: Check for mmap64 before using it
Created attachment 365868 [details] [review] v4l2object: Don't use mmap64 if off_t is 64-bit The difference between mmap and mmap64 is the type of 'offset' argument. mmap64 always uses a 64-bit interger as offset, while mmap uses off_t, whose size can vary on different operating systems or architectures. However, not all operating systems support mmap64. Fortunately, although FreeBSD only has mmap, its off_t is always 64-bit regardless of architectures, so we can simply use mmap when sizeof(off_t) == 8.
Sorry, I didn't check the status of the patch before uploading a new patch ...
Review of attachment 365868 [details] [review]: That patch looks better indeed, ok, I'll probably replace tomorrow then, thank you.
Attachment 365868 [details] pushed as 4026211 - v4l2object: Don't use mmap64 if off_t is 64-bit
I've reverted the previous one and replaced by this one. Thanks a lot of your work.
I have problem with mmap on Raspberry Pi 3. gstv4l2object.c: In function ‘gst_v4l2_object_new’: gstv4l2object.c:520:22: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types] v4l2object->mmap = mmap; ^ cc1: all warnings being treated as errors What could be the problem?
Maybe you could comment on https://bugzilla.gnome.org/show_bug.cgi?id=793103 and help fixing this.