Hello, Gregor Jasny wrote: > Hello, > > Due to complete lack of unit / integration tests I feel uncomfortable > merging this patch without the ACK of Hans de Goede. Thanks for merging the other patches. Sorry for them having been dependent on this patch. > On 05/05/15 21:02, Felix Janda wrote: > > Since _LARGEFILE64_SOURCE is 1, these types coincide if defined. > > This statement is only partially true: > > $ git grep _LARGEFILE64_SOURCE > lib/libv4l1/v4l1compat.c:#define _LARGEFILE64_SOURCE 1 > lib/libv4l2/v4l2convert.c:#define _LARGEFILE64_SOURCE 1 > > So LARGEFILE64_SOURCE will be only defined within the wrappers. > But libv4lsyscall-priv.h / SYS_MMAP is also used elsewhere. Actually, I think _LARGEFILE64_SOURCE does not influence whether _off_t is off_t (on 32bit) or not. What is rather needed is that _FILE_OFFSET_BITS is not 64. See e.g. http://www.freecode.com/articles/largefile-support-problems On the hand, I think that it would actually be benificial to have _FILE_OFFSET_BITS=64 globally so that on 32bit (glibc) systems all of v4l2-utils can deal with files >2GB. In the LD_PRELOAD libraries the special casing for glibc on linux would need to be changed. I'm preparing a patch for discussion. > But I wonder why SYS_MMAP is there in the first place? Maybe because in > the LD_PRELOAD case the default mmap symbol resolves to our wrapper? Exactly. First, syscall was used directly and later SYS_MMAP was introduced to compile on FreeBSD. > But in that case can't we gently ask the loader to give us the next > symbol in the chain via dlsym(RTLD_NEXT, "mmap")? This seems preferable to having to deal with the differences between Linux/FreeBSD and mmap/mmap2. For glibc on linux we should make sure that "mmap64" is used instead. We could use in the mmap wrapper a static variables to detect whether we should call v4l*_mmap or the mmap from libc. Felix -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
