On Tue, 13 Dec 2022 16:27:37 +0800
Gao Xiang <[email protected]> wrote:

> (cc Khem Raj <[email protected]>... Since I'd like to apply this first
>  and then enable _FILE_OFFSET_BITS=64 unconditionally so that we could
>  clean up all hard-coded _FILE_OFFSET_BITS in the source code.)
> 
> Hi Yue,
> 
> On Tue, Dec 13, 2022 at 03:16:43PM +0800, Yue Hu wrote:
> > From: Yue Hu <[email protected]>
> > 
> > The return value of ftell() is a long int, use ftello{64} instead. Also,
> > use lseek64 for large file position to avoid this error if we have it.
> > And need to return at once if they fail.
> > 
> > Signed-off-by: Yue Hu <[email protected]>
> > ---
> >  configure.ac    |  2 ++
> >  lib/fragments.c | 34 +++++++++++++++++++++++++++++-----
> >  lib/inode.c     |  9 +++++++--
> >  3 files changed, 38 insertions(+), 7 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index a736ff0..5c9666a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -190,6 +190,8 @@ AC_CHECK_FUNCS(m4_flatten([
> >     llistxattr
> >     memset
> >     realpath
> > +   lseek64
> > +   ftello64
> >     pread64
> >     pwrite64
> >     strdup
> > diff --git a/lib/fragments.c b/lib/fragments.c
> > index e69ae47..4302cd5 100644
> > --- a/lib/fragments.c
> > +++ b/lib/fragments.c
> > @@ -3,7 +3,18 @@
> >   * Copyright (C), 2022, Coolpad Group Limited.
> >   * Created by Yue Hu <[email protected]>
> >   */
> > +#ifndef _LARGEFILE_SOURCE
> > +#define _LARGEFILE_SOURCE
> > +#endif
> > +#ifndef _LARGEFILE64_SOURCE
> > +#define _LARGEFILE64_SOURCE
> > +#endif
> > +#ifndef _FILE_OFFSET_BITS
> > +#define _FILE_OFFSET_BITS 64
> > +#endif
> > +#ifndef _GNU_SOURCE
> >  #define _GNU_SOURCE
> > +#endif
> >  #include <stdlib.h>
> >  #include <unistd.h>
> >  #include "erofs/err.h"
> > @@ -30,6 +41,13 @@ static struct list_head dupli_frags[FRAGMENT_HASHSIZE];
> >  static FILE *packedfile;
> >  const char *frags_packedname = "packed_file";
> >  
> > +#ifndef HAVE_LSEEK64
> > +static inline off_t lseek64(int fd, u64 offset, int set)
> > +{
> > +   return lseek(fd, offset, set);
> > +}
> > +#endif  
> 
> Could we use another function name other than native lseek64?
> 
> > +
> >  static int z_erofs_fragments_dedupe_find(struct erofs_inode *inode, int fd,
> >                                      u32 crc)
> >  {
> > @@ -53,8 +71,7 @@ static int z_erofs_fragments_dedupe_find(struct 
> > erofs_inode *inode, int fd,
> >     if (!data)
> >             return -ENOMEM;
> >  
> > -   ret = lseek(fd, inode->i_size - length, SEEK_SET);
> > -   if (ret < 0) {
> > +   if (lseek64(fd, inode->i_size - length, SEEK_SET) < 0) {
> >             ret = -errno;
> >             goto out;
> >     }
> > @@ -113,8 +130,7 @@ int z_erofs_fragments_dedupe(struct erofs_inode *inode, 
> > int fd, u32 *tofcrc)
> >     if (inode->i_size <= EROFS_TOF_HASHLEN)
> >             return 0;
> >  
> > -   ret = lseek(fd, inode->i_size - EROFS_TOF_HASHLEN, SEEK_SET);
> > -   if (ret < 0)
> > +   if (lseek64(fd, inode->i_size - EROFS_TOF_HASHLEN, SEEK_SET) < 0)
> >             return -errno;
> >  
> >     ret = read(fd, data_to_hash, EROFS_TOF_HASHLEN);
> > @@ -192,9 +208,17 @@ void z_erofs_fragments_commit(struct erofs_inode 
> > *inode)
> >  int z_erofs_pack_fragments(struct erofs_inode *inode, void *data,
> >                        unsigned int len, u32 tofcrc)
> >  {
> > +#ifdef HAVE_FTELLO64
> > +   off64_t offset = ftello64(packedfile);
> > +#else
> > +   off_t offset = ftello(packedfile);
> > +#endif
> >     int ret;
> >  
> > -   inode->fragmentoff = ftell(packedfile);
> > +   if (offset < 0)
> > +           return -errno;
> > +
> > +   inode->fragmentoff = (erofs_off_t)offset;
> >     inode->fragment_size = len;
> >  
> >     if (fwrite(data, len, 1, packedfile) != 1)
> > diff --git a/lib/inode.c b/lib/inode.c
> > index 9de4dec..a8510f3 100644
> > --- a/lib/inode.c
> > +++ b/lib/inode.c
> > @@ -1196,7 +1196,10 @@ struct erofs_inode 
> > *erofs_mkfs_build_special_from_fd(int fd, const char *name)
> >     struct erofs_inode *inode;
> >     int ret;
> >  
> > -   lseek(fd, 0, SEEK_SET);
> > +   ret = lseek(fd, 0, SEEK_SET);
> > +   if (ret < 0)
> > +           return ERR_PTR(-errno);  
> 
> This should be in a seperate patch, please don't fix it here.
> 
> > +
> >     ret = fstat64(fd, &st);
> >     if (ret)
> >             return ERR_PTR(-errno);
> > @@ -1223,7 +1226,9 @@ struct erofs_inode 
> > *erofs_mkfs_build_special_from_fd(int fd, const char *name)
> >  
> >     ret = erofs_write_compressed_file(inode, fd);
> >     if (ret == -ENOSPC) {
> > -           lseek(fd, 0, SEEK_SET);
> > +           ret = lseek(fd, 0, SEEK_SET);
> > +           if (ret < 0)  
> 
> Same here.

Okay, will update for all in v2.

> 
> Thanks,
> Gao Xiang
> 
> > +                   return ERR_PTR(-errno);
> >             ret = write_uncompressed_file_from_fd(inode, fd);
> >     }
> >  
> > -- 
> > 2.17.1  

Reply via email to