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

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

[Felix Janda]

Since _LARGEFILE64_SOURCE is 1, these types coincide if defined.

Signed-off-by: Felix Janda <felix.janda@posteo.de>
---
 lib/libv4l1/v4l1compat.c               | 5 ++---
 lib/libv4l2/v4l2convert.c              | 4 ++--
 lib/libv4lconvert/libv4lsyscall-priv.h | 7 +++----
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
index e328288..07240c1 100644
--- a/lib/libv4l1/v4l1compat.c
+++ b/lib/libv4l1/v4l1compat.c
@@ -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);
 }
diff --git a/lib/libv4l2/v4l2convert.c b/lib/libv4l2/v4l2convert.c
index 9b46ab8..b65da5e 100644
--- a/lib/libv4l2/v4l2convert.c
+++ b/lib/libv4l2/v4l2convert.c
@@ -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);
 }
diff --git a/lib/libv4lconvert/libv4lsyscall-priv.h b/lib/libv4lconvert/libv4lsyscall-priv.h
index cdd38bc..ce89073 100644
--- a/lib/libv4lconvert/libv4lsyscall-priv.h
+++ b/lib/libv4lconvert/libv4lsyscall-priv.h
@@ -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) \
-- 
2.0.5

Re: [PATCH 1/4] Use off_t and off64_t instead of __off_t and __off64_t

[Mauro Carvalho Chehab]

Em Sun, 25 Jan 2015 21:36:15 +0100
Felix Janda <felix.janda@posteo.de> escreveu:

> 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



> 
> Signed-off-by: Felix Janda <felix.janda@posteo.de>
> ---
>  lib/libv4l1/v4l1compat.c               | 5 ++---
>  lib/libv4l2/v4l2convert.c              | 4 ++--
>  lib/libv4lconvert/libv4lsyscall-priv.h | 7 +++----
>  3 files changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
> index e328288..07240c1 100644
> --- a/lib/libv4l1/v4l1compat.c
> +++ b/lib/libv4l1/v4l1compat.c
> @@ -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);
>  }
> diff --git a/lib/libv4l2/v4l2convert.c b/lib/libv4l2/v4l2convert.c
> index 9b46ab8..b65da5e 100644
> --- a/lib/libv4l2/v4l2convert.c
> +++ b/lib/libv4l2/v4l2convert.c
> @@ -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);
>  }
> diff --git a/lib/libv4lconvert/libv4lsyscall-priv.h b/lib/libv4lconvert/libv4lsyscall-priv.h
> index cdd38bc..ce89073 100644
> --- a/lib/libv4lconvert/libv4lsyscall-priv.h
> +++ b/lib/libv4lconvert/libv4lsyscall-priv.h
> @@ -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) \

[PATCHv2 1/4] Use off_t and off64_t instead of __off_t and __off64_t

[Felix Janda]

Since _LARGEFILE64_SOURCE is 1, these types coincide if defined.

Signed-off-by: Felix Janda <felix.janda@posteo.de>
---
v2: rebased

Mauro Carvalho Chehab wrote:

Thanks for the review.

> Em Sun, 25 Jan 2015 21:36:15 +0100
> Felix Janda <felix.janda@posteo.de> escreveu:
> 
> > 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. 

> 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(-)

diff --git a/lib/libv4l1/v4l1compat.c b/lib/libv4l1/v4l1compat.c
index c641c17..282173b 100644
--- a/lib/libv4l1/v4l1compat.c
+++ b/lib/libv4l1/v4l1compat.c
@@ -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);
 }
diff --git a/lib/libv4l2/v4l2convert.c b/lib/libv4l2/v4l2convert.c
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);
 }
diff --git a/lib/libv4lconvert/libv4lsyscall-priv.h b/lib/libv4lconvert/libv4lsyscall-priv.h
index f548fb2..f87eff4 100644
--- a/lib/libv4lconvert/libv4lsyscall-priv.h
+++ b/lib/libv4lconvert/libv4lsyscall-priv.h
@@ -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) \
-- 
2.3.6

Re: [PATCHv2 1/4] Use off_t and off64_t instead of __off_t and __off64_t

[Gregor Jasny]

Hello,

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

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.

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

Re: [PATCHv2 1/4] Use off_t and off64_t instead of __off_t and __off64_t

[Felix Janda]

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