Use off_t and off64_t instead of __off_t and __off64_t

PATCH Superseded Felix Janda 3 participants 5 messages Jan 25, 2015 – May 10, 2015 Media (V4L/DVB)

lore

Share & tools

lore

Felix Janda

author

2015-01-25 20:37 UTC #0 lore raw

PATCH 1/4 Use off_t and off64_t instead of __off_t and __off64_t

Since _LARGEFILE64_SOURCE is 1, these types coincide if defined.

Signed-off-by Felix Janda <[email protected]>

---
 lib/libv4l1/v4l1compat.c               | 5 ++---
 lib/libv4l2/v4l2convert.c              | 4 ++--
 lib/libv4lconvert/libv4lsyscall-priv.h | 7 +++----
 3 files changed, 7 insertions(+), 9 deletions(-)

lib/libv4l1/v4l1compat.c +2 -3

index e328288..07240c1 100644
@@ -26,7 +26,6 @@
 #include <stdarg.h>
 #include <fcntl.h>
 #include <libv4l1.h>
-#include "../libv4lconvert/libv4lsyscall-priv.h" /* for __off_t */
 
 #include <sys/ioctl.h>
 #include <sys/mman.h>

@@ -112,14 +111,14 @@ LIBV4L_PUBLIC ssize_t read(int fd, void *buffer, size_t n)
 }
 
 LIBV4L_PUBLIC void *mmap(void *start, size_t length, int prot, int flags, int fd,
-		__off_t offset)
+		off_t offset)
 {
 	return v4l1_mmap(start, length, prot, flags, fd, offset);
 }
 
 #ifdef linux
 LIBV4L_PUBLIC void *mmap64(void *start, size_t length, int prot, int flags, int fd,
-		__off64_t offset)
+		off64_t offset)
 {
 	return v4l1_mmap(start, length, prot, flags, fd, offset);
 }

lib/libv4l2/v4l2convert.c +2 -2

index 9b46ab8..b65da5e 100644
@@ -139,14 +139,14 @@ LIBV4L_PUBLIC ssize_t read(int fd, void *buffer, size_t n)
 }
 
 LIBV4L_PUBLIC void *mmap(void *start, size_t length, int prot, int flags, int fd,
-		__off_t offset)
+		off_t offset)
 {
 	return v4l2_mmap(start, length, prot, flags, fd, offset);
 }
 
 #ifdef linux
 LIBV4L_PUBLIC void *mmap64(void *start, size_t length, int prot, int flags, int fd,
-		__off64_t offset)
+		off64_t offset)
 {
 	return v4l2_mmap(start, length, prot, flags, fd, offset);
 }

lib/libv4lconvert/libv4lsyscall-priv.h +3 -4

index cdd38bc..ce89073 100644
@@ -59,7 +59,6 @@
 #define	_IOC_SIZE(cmd) IOCPARM_LEN(cmd)
 #define	MAP_ANONYMOUS MAP_ANON
 #define	MMAP2_PAGE_SHIFT 0
-typedef off_t __off_t;
 #endif
 
 #undef SYS_OPEN

@@ -91,15 +90,15 @@ typedef off_t __off_t;
 #if defined(__FreeBSD__)
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	__syscall(SYS_mmap, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)(off))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)(off))
 #elif defined(__FreeBSD_kernel__)
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	syscall(SYS_mmap, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)(off))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)(off))
 #else
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	syscall(SYS_mmap2, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)((off) >> MMAP2_PAGE_SHIFT))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)((off) >> MMAP2_PAGE_SHIFT))
 #endif
 
 #define SYS_MUNMAP(addr, len) \

Signature · 1 line

2.0.5

Mauro Carvalho Chehab

2015-05-05 12:37 UTC #1 lore raw

Em Sun, 25 Jan 2015 21:36:15 +0100
Felix Janda <[email protected]> escreveu:

Quoted 1 line

Since _LARGEFILE64_SOURCE is 1, these types coincide if defined.

The __off_t macro was also added by the FreeBSD patchset. Removing this will likely break for FreeBSD.

So, provided that this is not causing any issues, better to keep it as-is.

Regards,
Mauro

Quoted 89 lines

Signed-off-by Felix Janda <[email protected]>

---
 lib/libv4l1/v4l1compat.c               | 5 ++---
 lib/libv4l2/v4l2convert.c              | 4 ++--
 lib/libv4lconvert/libv4lsyscall-priv.h | 7 +++----
 3 files changed, 7 insertions(+), 9 deletions(-)

lib/libv4l1/v4l1compat.c +2 -3

index e328288..07240c1 100644
@@ -26,7 +26,6 @@
 #include <stdarg.h>
 #include <fcntl.h>
 #include <libv4l1.h>
-#include "../libv4lconvert/libv4lsyscall-priv.h" /* for __off_t */
 
 #include <sys/ioctl.h>
 #include <sys/mman.h>

@@ -112,14 +111,14 @@ LIBV4L_PUBLIC ssize_t read(int fd, void *buffer, size_t n)
 }
 
 LIBV4L_PUBLIC void *mmap(void *start, size_t length, int prot, int flags, int fd,
-		__off_t offset)
+		off_t offset)
 {
 	return v4l1_mmap(start, length, prot, flags, fd, offset);
 }
 
 #ifdef linux
 LIBV4L_PUBLIC void *mmap64(void *start, size_t length, int prot, int flags, int fd,
-		__off64_t offset)
+		off64_t offset)
 {
 	return v4l1_mmap(start, length, prot, flags, fd, offset);
 }

lib/libv4l2/v4l2convert.c +2 -2

index 9b46ab8..b65da5e 100644
@@ -139,14 +139,14 @@ LIBV4L_PUBLIC ssize_t read(int fd, void *buffer, size_t n)
 }
 
 LIBV4L_PUBLIC void *mmap(void *start, size_t length, int prot, int flags, int fd,
-		__off_t offset)
+		off_t offset)
 {
 	return v4l2_mmap(start, length, prot, flags, fd, offset);
 }
 
 #ifdef linux
 LIBV4L_PUBLIC void *mmap64(void *start, size_t length, int prot, int flags, int fd,
-		__off64_t offset)
+		off64_t offset)
 {
 	return v4l2_mmap(start, length, prot, flags, fd, offset);
 }

lib/libv4lconvert/libv4lsyscall-priv.h +3 -4

index cdd38bc..ce89073 100644
@@ -59,7 +59,6 @@
 #define	_IOC_SIZE(cmd) IOCPARM_LEN(cmd)
 #define	MAP_ANONYMOUS MAP_ANON
 #define	MMAP2_PAGE_SHIFT 0
-typedef off_t __off_t;
 #endif
 
 #undef SYS_OPEN

@@ -91,15 +90,15 @@ typedef off_t __off_t;
 #if defined(__FreeBSD__)
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	__syscall(SYS_mmap, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)(off))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)(off))
 #elif defined(__FreeBSD_kernel__)
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	syscall(SYS_mmap, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)(off))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)(off))
 #else
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	syscall(SYS_mmap2, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)((off) >> MMAP2_PAGE_SHIFT))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)((off) >> MMAP2_PAGE_SHIFT))
 #endif
 
 #define SYS_MUNMAP(addr, len) \

Felix Janda

author

2015-05-05 19:02 UTC #2 lore raw

PATCH v2 1/4 Use off_t and off64_t instead of __off_t and __off64_t

Since _LARGEFILE64_SOURCE is 1, these types coincide if defined.

Signed-off-by Felix Janda <[email protected]>

---
v2: rebased

Mauro Carvalho Chehab wrote:

Thanks for the review.

Quoted 7 lines

Em Sun, 25 Jan 2015 21:36:15 +0100
Felix Janda <[email protected]> escreveu:
Quoted 1 line
Since _LARGEFILE64_SOURCE is 1, these types coincide if defined.
The __off_t macro was also added by the FreeBSD patchset. Removing this will likely break for FreeBSD.

No. The patchset added a typedef of __off_t to off_t. Why? Because FreeBSD does not have off_t. By now for a similar reason there is a similar typedef for android. Generally, names starting with one or two underscores are implementation specific and not portable. If the source code did not use __off_t, there would be no need to introduce these typedefs.

This patch removes all occurences of __off_t and __off64_t in the source code.

Quoted 2 lines

So, provided that this is not causing any issues, better to keep it as-is.

It produces "error: unknown type name '__off_t'" on musl based linux systems.

---
 lib/libv4l1/v4l1compat.c               |  5 ++---
 lib/libv4l2/v4l2convert.c              |  5 ++---
 lib/libv4lconvert/libv4lsyscall-priv.h | 11 +++--------
 3 files changed, 7 insertions(+), 14 deletions(-)

lib/libv4l1/v4l1compat.c +2 -2

index c641c17..282173b 100644
@@ -116,14 +116,14 @@ LIBV4L_PUBLIC ssize_t read(int fd, void *buffer, size_t n)
 }
 
 LIBV4L_PUBLIC void *mmap(void *start, size_t length, int prot, int flags, int fd,
-		__off_t offset)
+		off_t offset)
 {
 	return v4l1_mmap(start, length, prot, flags, fd, offset);
 }
 
 #ifdef linux
 LIBV4L_PUBLIC void *mmap64(void *start, size_t length, int prot, int flags, int fd,
-		__off64_t offset)
+		off64_t offset)
 {
 	return v4l1_mmap(start, length, prot, flags, fd, offset);
 }

lib/libv4l2/v4l2convert.c +2 -2

index 008408e..2c2f12a 100644
@@ -148,14 +148,14 @@ LIBV4L_PUBLIC ssize_t read(int fd, void *buffer, size_t n)
 }
 
 LIBV4L_PUBLIC void *mmap(void *start, size_t length, int prot, int flags, int fd,
-		__off_t offset)
+		off_t offset)
 {
 	return v4l2_mmap(start, length, prot, flags, fd, offset);
 }
 
 #ifdef linux
 LIBV4L_PUBLIC void *mmap64(void *start, size_t length, int prot, int flags, int fd,
-		__off64_t offset)
+		off64_t offset)
 {
 	return v4l2_mmap(start, length, prot, flags, fd, offset);
 }

lib/libv4lconvert/libv4lsyscall-priv.h +3 -8

index f548fb2..f87eff4 100644
@@ -59,11 +59,6 @@
 #define	_IOC_SIZE(cmd) IOCPARM_LEN(cmd)
 #define	MAP_ANONYMOUS MAP_ANON
 #define	MMAP2_PAGE_SHIFT 0
-typedef off_t __off_t;
-#endif
-
-#if defined(ANDROID)
-typedef off_t __off_t;
 #endif
 
 #undef SYS_OPEN

@@ -95,15 +90,15 @@ typedef off_t __off_t;
 #if defined(__FreeBSD__)
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	__syscall(SYS_mmap, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)(off))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)(off))
 #elif defined(__FreeBSD_kernel__)
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	syscall(SYS_mmap, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)(off))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)(off))
 #else
 #define SYS_MMAP(addr, len, prot, flags, fd, off) \
 	syscall(SYS_mmap2, (void *)(addr), (size_t)(len), \
-			(int)(prot), (int)(flags), (int)(fd), (__off_t)((off) >> MMAP2_PAGE_SHIFT))
+			(int)(prot), (int)(flags), (int)(fd), (off_t)((off) >> MMAP2_PAGE_SHIFT))
 #endif
 
 #define SYS_MUNMAP(addr, len) \

Signature · 1 line

2.3.6

Gregor Jasny

2015-05-09 20:51 UTC #3 lore raw

Hello,

Due to complete lack of unit / integration tests I feel uncomfortable merging this patch without the ACK of Hans de Goede.

Quoted 1 line · Felix Janda

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.

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? 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")?

Thanks,
Gregor

Felix Janda

author

2015-05-10 10:53 UTC #4 lore raw

Hello,

Quoted 4 lines · Gregor Jasny

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.

Quoted 11 lines · Felix Janda

Quoted 1 line
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.

Quoted 2 lines

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.

Quoted 2 lines

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