[PATCH 1/3] configure: use AC_SYS_LARGEFILE

[PATCH 1/3] configure: use AC_SYS_LARGEFILE

[Khem Raj]

The autoconf macro AC_SYS_LARGEFILE defines _FILE_OFFSET_BITS=64
where necessary to ensure that off_t and all interfaces using off_t
are 64bit, even on 32bit systems.

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 configure.ac | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure.ac b/configure.ac
index a736ff0..b880bb0 100644
--- a/configure.ac
+++ b/configure.ac
@@ -13,6 +13,8 @@ AC_CONFIG_MACRO_DIR([m4])
 AC_CONFIG_AUX_DIR(config)
 AM_INIT_AUTOMAKE([foreign -Wall])
 
+AC_SYS_LARGEFILE
+
 # Checks for programs.
 AM_PROG_AR
 AC_PROG_CC
-- 
2.38.1

[PATCH 2/3] erofs_fs.h: Make LFS mandatory for all usecases

[Khem Raj]

erosfs depend on the consistent use of a 64bit offset
type, force downstreams to use transparent LFS (_FILE_OFFSET_BITS=64),
so that it becomes impossible for them to use 32bit interfaces.

include autoconf'ed config.h to get definition of _FILE_OFFSET_BITS
which was detected by configure. This header needs to be included
before any system headers are included to ensure they see the correct
definition of _FILE_OFFSET_BITS for the platform

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 dump/main.c              | 1 +
 fsck/main.c              | 1 +
 include/erofs/internal.h | 1 +
 include/erofs_fs.h       | 6 ++++++
 lib/Makefile.am          | 2 +-
 mkfs/main.c              | 1 +
 6 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/dump/main.c b/dump/main.c
index 49ff2b7..1b40f2a 100644
--- a/dump/main.c
+++ b/dump/main.c
@@ -6,6 +6,7 @@
  *            Guo Xuenan <guoxuenan@huawei.com>
  */
 #define _GNU_SOURCE
+#include "config.h"
 #include <stdlib.h>
 #include <getopt.h>
 #include <time.h>
diff --git a/fsck/main.c b/fsck/main.c
index 5a2f659..82eef93 100644
--- a/fsck/main.c
+++ b/fsck/main.c
@@ -3,6 +3,7 @@
  * Copyright 2021 Google LLC
  * Author: Daeho Jeong <daehojeong@google.com>
  */
+#include "config.h"
 #include <stdlib.h>
 #include <getopt.h>
 #include <time.h>
diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index 6a70f11..9cc20a8 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -12,6 +12,7 @@ extern "C"
 {
 #endif
 
+#include <config.h>
 #include "list.h"
 #include "err.h"
 
diff --git a/include/erofs_fs.h b/include/erofs_fs.h
index 08f9761..a3bd93c 100644
--- a/include/erofs_fs.h
+++ b/include/erofs_fs.h
@@ -9,6 +9,8 @@
 #ifndef __EROFS_FS_H
 #define __EROFS_FS_H
 
+#include <sys/types.h>
+
 #define EROFS_SUPER_MAGIC_V1    0xE0F5E1E2
 #define EROFS_SUPER_OFFSET      1024
 
@@ -410,6 +412,10 @@ enum {
 
 #define EROFS_NAME_LEN      255
 
+
+/* make sure that any user of the erofs headers has atleast 64bit off_t type */
+extern int eros_assert_largefile[sizeof(off_t)-8];
+
 /* check the EROFS on-disk layout strictly at compile time */
 static inline void erofs_check_ondisk_layout_definitions(void)
 {
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 3fad357..88400ed 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -28,7 +28,7 @@ noinst_HEADERS += compressor.h
 liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
 		      namei.c data.c compress.c compressor.c zmap.c decompress.c \
 		      compress_hints.c hashmap.c sha256.c blobchunk.c dir.c
-liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
+liberofs_la_CFLAGS = -Wall -I$(top_builddir) -I$(top_srcdir)/include -include config.h
 if ENABLE_LZ4
 liberofs_la_CFLAGS += ${LZ4_CFLAGS}
 liberofs_la_SOURCES += compressor_lz4.c
diff --git a/mkfs/main.c b/mkfs/main.c
index d2c9830..0e601d9 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -5,6 +5,7 @@
  * Created by Li Guifu <bluce.liguifu@huawei.com>
  */
 #define _GNU_SOURCE
+#include "config.h"
 #include <time.h>
 #include <sys/time.h>
 #include <stdlib.h>
-- 
2.38.1

[PATCH 3/3] erosfs: replace [l]stat64 by equivalent [l]stat

[Khem Raj]

Signed-off-by: Khem Raj <raj.khem@gmail.com>
---
 lib/inode.c | 10 +++++-----
 lib/xattr.c |  4 ++--
 mkfs/main.c |  4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/lib/inode.c b/lib/inode.c
index f192510..38003fc 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -773,7 +773,7 @@ static u32 erofs_new_encode_dev(dev_t dev)
 
 #ifdef WITH_ANDROID
 int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
-			       struct stat64 *st,
+			       struct stat *st,
 			       const char *path)
 {
 	/* filesystem_config does not preserve file type bits */
@@ -818,7 +818,7 @@ int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
 }
 #else
 static int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
-				      struct stat64 *st,
+				      struct stat *st,
 				      const char *path)
 {
 	return 0;
@@ -826,7 +826,7 @@ static int erofs_droid_inode_fsconfig(struct erofs_inode *inode,
 #endif
 
 static int erofs_fill_inode(struct erofs_inode *inode,
-			    struct stat64 *st,
+			    struct stat *st,
 			    const char *path)
 {
 	int err = erofs_droid_inode_fsconfig(inode, st, path);
@@ -910,7 +910,7 @@ static struct erofs_inode *erofs_new_inode(void)
 /* get the inode from the (source) path */
 static struct erofs_inode *erofs_iget_from_path(const char *path, bool is_src)
 {
-	struct stat64 st;
+	struct stat st;
 	struct erofs_inode *inode;
 	int ret;
 
@@ -918,7 +918,7 @@ static struct erofs_inode *erofs_iget_from_path(const char *path, bool is_src)
 	if (!is_src)
 		return ERR_PTR(-EINVAL);
 
-	ret = lstat64(path, &st);
+	ret = lstat(path, &st);
 	if (ret)
 		return ERR_PTR(-errno);
 
diff --git a/lib/xattr.c b/lib/xattr.c
index 71ffe3e..fd0e728 100644
--- a/lib/xattr.c
+++ b/lib/xattr.c
@@ -467,7 +467,7 @@ static int erofs_count_all_xattrs_from_path(const char *path)
 {
 	int ret;
 	DIR *_dir;
-	struct stat64 st;
+	struct stat st;
 
 	_dir = opendir(path);
 	if (!_dir) {
@@ -502,7 +502,7 @@ static int erofs_count_all_xattrs_from_path(const char *path)
 			goto fail;
 		}
 
-		ret = lstat64(buf, &st);
+		ret = lstat(buf, &st);
 		if (ret) {
 			ret = -errno;
 			goto fail;
diff --git a/mkfs/main.c b/mkfs/main.c
index 0e601d9..ca1ac16 100644
--- a/mkfs/main.c
+++ b/mkfs/main.c
@@ -582,7 +582,7 @@ int main(int argc, char **argv)
 	struct erofs_buffer_head *sb_bh;
 	struct erofs_inode *root_inode;
 	erofs_nid_t root_nid;
-	struct stat64 st;
+	struct stat st;
 	erofs_blk_t nblocks;
 	struct timeval t;
 	char uuid_str[37] = "not available";
@@ -610,7 +610,7 @@ int main(int argc, char **argv)
 			return 1;
 	}
 
-	err = lstat64(cfg.c_src_path, &st);
+	err = lstat(cfg.c_src_path, &st);
 	if (err)
 		return 1;
 	if (!S_ISDIR(st.st_mode)) {
-- 
2.38.1

Re: [PATCH 2/3] erofs_fs.h: Make LFS mandatory for all usecases

[Gao Xiang]

Hi Khem,

On Thu, Dec 08, 2022 at 12:53:34AM -0800, Khem Raj wrote:
> erosfs depend on the consistent use of a 64bit offset

Thanks for your patch!

  ^ erofs

> type, force downstreams to use transparent LFS (_FILE_OFFSET_BITS=64),
> so that it becomes impossible for them to use 32bit interfaces.
> 
> include autoconf'ed config.h to get definition of _FILE_OFFSET_BITS
> which was detected by configure. This header needs to be included
> before any system headers are included to ensure they see the correct
> definition of _FILE_OFFSET_BITS for the platform
> 
> Signed-off-by: Khem Raj <raj.khem@gmail.com>
> ---

...

> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index 6a70f11..9cc20a8 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -12,6 +12,7 @@ extern "C"
>  {
>  #endif
>  
> +#include <config.h>

could we use alternative way? since I'd like to make include/ as
liberofs later, and "config.h" autoconf seems weird to me...

>  #include "list.h"
>  #include "err.h"
>  
> diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> index 08f9761..a3bd93c 100644
> --- a/include/erofs_fs.h
> +++ b/include/erofs_fs.h
> @@ -9,6 +9,8 @@
>  #ifndef __EROFS_FS_H
>  #define __EROFS_FS_H
>  
> +#include <sys/types.h>

Could you give more hints why we need this here? 

> +
>  #define EROFS_SUPER_MAGIC_V1    0xE0F5E1E2
>  #define EROFS_SUPER_OFFSET      1024
>  
> @@ -410,6 +412,10 @@ enum {
>  
>  #define EROFS_NAME_LEN      255
>  
> +
> +/* make sure that any user of the erofs headers has atleast 64bit off_t type */
> +extern int eros_assert_largefile[sizeof(off_t)-8];

erofs? also you could add this into erofs/internal.h...

This file is just the on-disk definition...

> +
>  /* check the EROFS on-disk layout strictly at compile time */
>  static inline void erofs_check_ondisk_layout_definitions(void)
>  {
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 3fad357..88400ed 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -28,7 +28,7 @@ noinst_HEADERS += compressor.h
>  liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
>  		      namei.c data.c compress.c compressor.c zmap.c decompress.c \
>  		      compress_hints.c hashmap.c sha256.c blobchunk.c dir.c
> -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
> +liberofs_la_CFLAGS = -Wall -I$(top_builddir) -I$(top_srcdir)/include -include config.h

same here too...

Thanks,
Gao Xiang

>  if ENABLE_LZ4
>  liberofs_la_CFLAGS += ${LZ4_CFLAGS}
>  liberofs_la_SOURCES += compressor_lz4.c
> diff --git a/mkfs/main.c b/mkfs/main.c
> index d2c9830..0e601d9 100644
> --- a/mkfs/main.c
> +++ b/mkfs/main.c
> @@ -5,6 +5,7 @@
>   * Created by Li Guifu <bluce.liguifu@huawei.com>
>   */
>  #define _GNU_SOURCE
> +#include "config.h"
>  #include <time.h>
>  #include <sys/time.h>
>  #include <stdlib.h>
> -- 
> 2.38.1
> 

Re: [PATCH 2/3] erofs_fs.h: Make LFS mandatory for all usecases

[Khem Raj]

On Thu, Dec 8, 2022 at 6:18 AM Gao Xiang <xiang@kernel.org> wrote:
>
> Hi Khem,
>
> On Thu, Dec 08, 2022 at 12:53:34AM -0800, Khem Raj wrote:
> > erosfs depend on the consistent use of a 64bit offset
>
> Thanks for your patch!
>
>   ^ erofs

Done in v2

>
> > type, force downstreams to use transparent LFS (_FILE_OFFSET_BITS=64),
> > so that it becomes impossible for them to use 32bit interfaces.
> >
> > include autoconf'ed config.h to get definition of _FILE_OFFSET_BITS
> > which was detected by configure. This header needs to be included
> > before any system headers are included to ensure they see the correct
> > definition of _FILE_OFFSET_BITS for the platform
> >
> > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > ---
>
> ...
>
> > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > index 6a70f11..9cc20a8 100644
> > --- a/include/erofs/internal.h
> > +++ b/include/erofs/internal.h
> > @@ -12,6 +12,7 @@ extern "C"
> >  {
> >  #endif
> >
> > +#include <config.h>
>
> could we use alternative way? since I'd like to make include/ as
> liberofs later, and "config.h" autoconf seems weird to me...
>

I am using the AC_SYS_LARGEFILE macro from autoconf to enable support for
largefile support during configure. configure will generate config.h
in build dir which
will contain the essential macros which we use e.g. _FILE_OFFSET_BITS defined
to right values. Alternate way is to pass it _always_ or demand it to
be passed from
user. Which in a way it will do with internal.h check added in this
series. I am fine
if you do not want to depend on autoconf support to enable LFS. Let me know.

> >  #include "list.h"
> >  #include "err.h"
> >
> > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > index 08f9761..a3bd93c 100644
> > --- a/include/erofs_fs.h
> > +++ b/include/erofs_fs.h
> > @@ -9,6 +9,8 @@
> >  #ifndef __EROFS_FS_H
> >  #define __EROFS_FS_H
> >
> > +#include <sys/types.h>
>
> Could you give more hints why we need this here?

Its needed to get off_t defined, I have added a comment here
in v2
>
> > +
> >  #define EROFS_SUPER_MAGIC_V1    0xE0F5E1E2
> >  #define EROFS_SUPER_OFFSET      1024
> >
> > @@ -410,6 +412,10 @@ enum {
> >
> >  #define EROFS_NAME_LEN      255
> >
> > +
> > +/* make sure that any user of the erofs headers has atleast 64bit off_t type */
> > +extern int eros_assert_largefile[sizeof(off_t)-8];
>
> erofs? also you could add this into erofs/internal.h...
>
> This file is just the on-disk definition...

yeah moved the check to internal.h in v2

>
> > +
> >  /* check the EROFS on-disk layout strictly at compile time */
> >  static inline void erofs_check_ondisk_layout_definitions(void)
> >  {
> > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > index 3fad357..88400ed 100644
> > --- a/lib/Makefile.am
> > +++ b/lib/Makefile.am
> > @@ -28,7 +28,7 @@ noinst_HEADERS += compressor.h
> >  liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
> >                     namei.c data.c compress.c compressor.c zmap.c decompress.c \
> >                     compress_hints.c hashmap.c sha256.c blobchunk.c dir.c
> > -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
> > +liberofs_la_CFLAGS = -Wall -I$(top_builddir) -I$(top_srcdir)/include -include config.h
>
> same here too...

as said above if we are ok to pass it always then we can add -D
_FILE_OFFSET_BITS=64 via toplevel Makefile.am
it will only be needed on 32bit systems though, so maybe we do not
define it and demand it from users via CFLAGS
if they compile it for 32bit systems.

>
> Thanks,
> Gao Xiang
>
> >  if ENABLE_LZ4
> >  liberofs_la_CFLAGS += ${LZ4_CFLAGS}
> >  liberofs_la_SOURCES += compressor_lz4.c
> > diff --git a/mkfs/main.c b/mkfs/main.c
> > index d2c9830..0e601d9 100644
> > --- a/mkfs/main.c
> > +++ b/mkfs/main.c
> > @@ -5,6 +5,7 @@
> >   * Created by Li Guifu <bluce.liguifu@huawei.com>
> >   */
> >  #define _GNU_SOURCE
> > +#include "config.h"
> >  #include <time.h>
> >  #include <sys/time.h>
> >  #include <stdlib.h>
> > --
> > 2.38.1
> >

Re: [PATCH 2/3] erofs_fs.h: Make LFS mandatory for all usecases

[Gao Xiang]

Hi Khem,

On Fri, Dec 09, 2022 at 06:20:12PM -0800, Khem Raj wrote:
> On Thu, Dec 8, 2022 at 6:18 AM Gao Xiang <xiang@kernel.org> wrote:
> >
> > Hi Khem,
> >
> > On Thu, Dec 08, 2022 at 12:53:34AM -0800, Khem Raj wrote:
> > > erosfs depend on the consistent use of a 64bit offset
> >
> > Thanks for your patch!
> >
> >   ^ erofs
> 
> Done in v2
> 
> >
> > > type, force downstreams to use transparent LFS (_FILE_OFFSET_BITS=64),
> > > so that it becomes impossible for them to use 32bit interfaces.
> > >
> > > include autoconf'ed config.h to get definition of _FILE_OFFSET_BITS
> > > which was detected by configure. This header needs to be included
> > > before any system headers are included to ensure they see the correct
> > > definition of _FILE_OFFSET_BITS for the platform
> > >
> > > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > > ---
> >
> > ...
> >
> > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > index 6a70f11..9cc20a8 100644
> > > --- a/include/erofs/internal.h
> > > +++ b/include/erofs/internal.h
> > > @@ -12,6 +12,7 @@ extern "C"
> > >  {
> > >  #endif
> > >
> > > +#include <config.h>
> >
> > could we use alternative way? since I'd like to make include/ as
> > liberofs later, and "config.h" autoconf seems weird to me...
> >
> 
> I am using the AC_SYS_LARGEFILE macro from autoconf to enable support for
> largefile support during configure. configure will generate config.h
> in build dir which
> will contain the essential macros which we use e.g. _FILE_OFFSET_BITS defined
> to right values. Alternate way is to pass it _always_ or demand it to
> be passed from
> user. Which in a way it will do with internal.h check added in this
> series. I am fine
> if you do not want to depend on autoconf support to enable LFS. Let me know.
> 
> > >  #include "list.h"
> > >  #include "err.h"
> > >
> > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > index 08f9761..a3bd93c 100644
> > > --- a/include/erofs_fs.h
> > > +++ b/include/erofs_fs.h
> > > @@ -9,6 +9,8 @@
> > >  #ifndef __EROFS_FS_H
> > >  #define __EROFS_FS_H
> > >
> > > +#include <sys/types.h>
> >
> > Could you give more hints why we need this here?
> 
> Its needed to get off_t defined, I have added a comment here
> in v2i

but we don't use off_t in erofs_fs.h?

> >
> > > +
> > >  #define EROFS_SUPER_MAGIC_V1    0xE0F5E1E2
> > >  #define EROFS_SUPER_OFFSET      1024
> > >
> > > @@ -410,6 +412,10 @@ enum {
> > >
> > >  #define EROFS_NAME_LEN      255
> > >
> > > +
> > > +/* make sure that any user of the erofs headers has atleast 64bit off_t type */
> > > +extern int eros_assert_largefile[sizeof(off_t)-8];
> >
> > erofs? also you could add this into erofs/internal.h...
> >
> > This file is just the on-disk definition...
> 
> yeah moved the check to internal.h in v2
> 
> >
> > > +
> > >  /* check the EROFS on-disk layout strictly at compile time */
> > >  static inline void erofs_check_ondisk_layout_definitions(void)
> > >  {
> > > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > > index 3fad357..88400ed 100644
> > > --- a/lib/Makefile.am
> > > +++ b/lib/Makefile.am
> > > @@ -28,7 +28,7 @@ noinst_HEADERS += compressor.h
> > >  liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
> > >                     namei.c data.c compress.c compressor.c zmap.c decompress.c \
> > >                     compress_hints.c hashmap.c sha256.c blobchunk.c dir.c
> > > -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
> > > +liberofs_la_CFLAGS = -Wall -I$(top_builddir) -I$(top_srcdir)/include -include config.h
> >
> > same here too...
> 
> as said above if we are ok to pass it always then we can add -D
> _FILE_OFFSET_BITS=64 via toplevel Makefile.am
> it will only be needed on 32bit systems though, so maybe we do not
> define it and demand it from users via CFLAGS
> if they compile it for 32bit systems.
> 

I think use -D _FILE_OFFSET_BITS=64 would be a better choice...

Thanks,
Gao Xiang

Re: [PATCH 2/3] erofs_fs.h: Make LFS mandatory for all usecases

[Khem Raj]

On Sat, Dec 10, 2022 at 7:03 AM Gao Xiang <xiang@kernel.org> wrote:
>
> Hi Khem,
>
> On Fri, Dec 09, 2022 at 06:20:12PM -0800, Khem Raj wrote:
> > On Thu, Dec 8, 2022 at 6:18 AM Gao Xiang <xiang@kernel.org> wrote:
> > >
> > > Hi Khem,
> > >
> > > On Thu, Dec 08, 2022 at 12:53:34AM -0800, Khem Raj wrote:
> > > > erosfs depend on the consistent use of a 64bit offset
> > >
> > > Thanks for your patch!
> > >
> > >   ^ erofs
> >
> > Done in v2
> >
> > >
> > > > type, force downstreams to use transparent LFS (_FILE_OFFSET_BITS=64),
> > > > so that it becomes impossible for them to use 32bit interfaces.
> > > >
> > > > include autoconf'ed config.h to get definition of _FILE_OFFSET_BITS
> > > > which was detected by configure. This header needs to be included
> > > > before any system headers are included to ensure they see the correct
> > > > definition of _FILE_OFFSET_BITS for the platform
> > > >
> > > > Signed-off-by: Khem Raj <raj.khem@gmail.com>
> > > > ---
> > >
> > > ...
> > >
> > > > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > > > index 6a70f11..9cc20a8 100644
> > > > --- a/include/erofs/internal.h
> > > > +++ b/include/erofs/internal.h
> > > > @@ -12,6 +12,7 @@ extern "C"
> > > >  {
> > > >  #endif
> > > >
> > > > +#include <config.h>
> > >
> > > could we use alternative way? since I'd like to make include/ as
> > > liberofs later, and "config.h" autoconf seems weird to me...
> > >
> >
> > I am using the AC_SYS_LARGEFILE macro from autoconf to enable support for
> > largefile support during configure. configure will generate config.h
> > in build dir which
> > will contain the essential macros which we use e.g. _FILE_OFFSET_BITS defined
> > to right values. Alternate way is to pass it _always_ or demand it to
> > be passed from
> > user. Which in a way it will do with internal.h check added in this
> > series. I am fine
> > if you do not want to depend on autoconf support to enable LFS. Let me know.
> >
> > > >  #include "list.h"
> > > >  #include "err.h"
> > > >
> > > > diff --git a/include/erofs_fs.h b/include/erofs_fs.h
> > > > index 08f9761..a3bd93c 100644
> > > > --- a/include/erofs_fs.h
> > > > +++ b/include/erofs_fs.h
> > > > @@ -9,6 +9,8 @@
> > > >  #ifndef __EROFS_FS_H
> > > >  #define __EROFS_FS_H
> > > >
> > > > +#include <sys/types.h>
> > >
> > > Could you give more hints why we need this here?
> >
> > Its needed to get off_t defined, I have added a comment here
> > in v2i
>
> but we don't use off_t in erofs_fs.h?

the new check I added does use it.

>
> > >
> > > > +
> > > >  #define EROFS_SUPER_MAGIC_V1    0xE0F5E1E2
> > > >  #define EROFS_SUPER_OFFSET      1024
> > > >
> > > > @@ -410,6 +412,10 @@ enum {
> > > >
> > > >  #define EROFS_NAME_LEN      255
> > > >
> > > > +
> > > > +/* make sure that any user of the erofs headers has atleast 64bit off_t type */
> > > > +extern int eros_assert_largefile[sizeof(off_t)-8];
> > >
> > > erofs? also you could add this into erofs/internal.h...
> > >
> > > This file is just the on-disk definition...
> >
> > yeah moved the check to internal.h in v2
> >
> > >
> > > > +
> > > >  /* check the EROFS on-disk layout strictly at compile time */
> > > >  static inline void erofs_check_ondisk_layout_definitions(void)
> > > >  {
> > > > diff --git a/lib/Makefile.am b/lib/Makefile.am
> > > > index 3fad357..88400ed 100644
> > > > --- a/lib/Makefile.am
> > > > +++ b/lib/Makefile.am
> > > > @@ -28,7 +28,7 @@ noinst_HEADERS += compressor.h
> > > >  liberofs_la_SOURCES = config.c io.c cache.c super.c inode.c xattr.c exclude.c \
> > > >                     namei.c data.c compress.c compressor.c zmap.c decompress.c \
> > > >                     compress_hints.c hashmap.c sha256.c blobchunk.c dir.c
> > > > -liberofs_la_CFLAGS = -Wall -I$(top_srcdir)/include
> > > > +liberofs_la_CFLAGS = -Wall -I$(top_builddir) -I$(top_srcdir)/include -include config.h
> > >
> > > same here too...
> >
> > as said above if we are ok to pass it always then we can add -D
> > _FILE_OFFSET_BITS=64 via toplevel Makefile.am
> > it will only be needed on 32bit systems though, so maybe we do not
> > define it and demand it from users via CFLAGS
> > if they compile it for 32bit systems.
> >
>
> I think use -D _FILE_OFFSET_BITS=64 would be a better choice...
>

OK I will rework it in v3.

> Thanks,
> Gao Xiang

Re: [PATCH 2/3] erofs_fs.h: Make LFS mandatory for all usecases

[Gao Xiang]

Hi Khem,

On Sat, Dec 10, 2022 at 11:49:25AM -0800, Khem Raj wrote:

...

> > >
> > > as said above if we are ok to pass it always then we can add -D
> > > _FILE_OFFSET_BITS=64 via toplevel Makefile.am
> > > it will only be needed on 32bit systems though, so maybe we do not
> > > define it and demand it from users via CFLAGS
> > > if they compile it for 32bit systems.
> > >
> >
> > I think use -D _FILE_OFFSET_BITS=64 would be a better choice...
> >
> 
> OK I will rework it in v3.

Do you still have some interest to work on this?
(I'm about to release erofs-utils 1.6 in a month.. It would be helpful
 to clean up such stuff in this version.)

Thanks,
Gao Xiang

> 
> > Thanks,
> > Gao Xiang

Re: [PATCH 2/3] erofs_fs.h: Make LFS mandatory for all usecases

[Khem Raj]

On Wed, Dec 14, 2022 at 8:12 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> Hi Khem,
>
> On Sat, Dec 10, 2022 at 11:49:25AM -0800, Khem Raj wrote:
>
> ...
>
> > > >
> > > > as said above if we are ok to pass it always then we can add -D
> > > > _FILE_OFFSET_BITS=64 via toplevel Makefile.am
> > > > it will only be needed on 32bit systems though, so maybe we do not
> > > > define it and demand it from users via CFLAGS
> > > > if they compile it for 32bit systems.
> > > >
> > >
> > > I think use -D _FILE_OFFSET_BITS=64 would be a better choice...
> > >
> >
> > OK I will rework it in v3.
>
> Do you still have some interest to work on this?
> (I'm about to release erofs-utils 1.6 in a month.. It would be helpful
>  to clean up such stuff in this version.)

Yes, I am testing a build of v3 today, and I will send out the v3
patchset soon after that.

>
> Thanks,
> Gao Xiang
>
> >
> > > Thanks,
> > > Gao Xiang