All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 1/2] Add some comments about const-ness around iterate API
@ 2021-12-21 14:28 Kelvin Zhang via Linux-erofs
  2021-12-21 14:28 ` [PATCH v1 2/2] Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
  2021-12-22  1:44 ` [PATCH v1 1/2] Add some comments about const-ness around iterate API Gao Xiang
  0 siblings, 2 replies; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2021-12-21 14:28 UTC (permalink / raw)
  To: linux-erofs mailing list, Miao Xie, Fang Wei; +Cc: Kelvin Zhang

Change-Id: I297a56ba14a37ef5eced95330a5b09109378ca44
Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
---
 include/erofs/dir.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/erofs/dir.h b/include/erofs/dir.h
index 77656ca..59bd40d 100644
--- a/include/erofs/dir.h
+++ b/include/erofs/dir.h
@@ -39,6 +39,14 @@ typedef int (*erofs_readdir_cb)(struct erofs_dir_context *);
  * the callback context. |de_namelen| is the exact dirent name length.
  */
 struct erofs_dir_context {
+	/* During execution of |erofs_iterate_dir|, the function needs
+	 * to read the values inside |erofs_inode* dir|. So it is important
+	 * that the callback function does not modify stuct pointed by
+	 * |dir|. It is OK to repoint |dir| to other objects.
+	 * Unfortunately, it's not possible to enforce this restriction
+	 * with const keyword, as |erofs_iterate_dir| needs to modify
+	 * struct pointed by |dir|.
+	 */
 	struct erofs_inode *dir;
 	erofs_readdir_cb cb;
 	erofs_nid_t pnid;		/* optional */
-- 
2.34.1.307.g9b7440fafd-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v1 2/2] Add API to get on disk size of an inode
  2021-12-21 14:28 [PATCH v1 1/2] Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
@ 2021-12-21 14:28 ` Kelvin Zhang via Linux-erofs
  2021-12-22  1:46   ` Gao Xiang
  2021-12-22  1:44 ` [PATCH v1 1/2] Add some comments about const-ness around iterate API Gao Xiang
  1 sibling, 1 reply; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2021-12-21 14:28 UTC (permalink / raw)
  To: linux-erofs mailing list, Miao Xie, Fang Wei; +Cc: Kelvin Zhang

Change-Id: I60fa9346737b14418bd3fa1d12f760aaf0a0cca5
Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
---
 dump/main.c              |  4 ++--
 include/erofs/internal.h |  2 ++
 lib/data.c               | 21 +++++++++++++++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/dump/main.c b/dump/main.c
index 71b44b4..cdde561 100644
--- a/dump/main.c
+++ b/dump/main.c
@@ -175,7 +175,7 @@ static int erofsdump_parse_options_cfg(int argc, char **argv)
 	return 0;
 }
 
-static int erofs_get_occupied_size(struct erofs_inode *inode,
+static int dump_get_occupied_size(struct erofs_inode *inode,
 		erofs_off_t *size)
 {
 	*size = 0;
@@ -291,7 +291,7 @@ static int erofs_read_dirent(struct erofs_dirent *de,
 		return err;
 	}
 
-	err = erofs_get_occupied_size(&inode, &occupied_size);
+	err = dump_get_occupied_size(&inode, &occupied_size);
 	if (err) {
 		erofs_err("get file size failed\n");
 		return err;
diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index 947304f..8f13e69 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -320,6 +320,8 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
 int erofs_map_blocks(struct erofs_inode *inode,
 		struct erofs_map_blocks *map, int flags);
 int erofs_map_dev(struct erofs_sb_info *sbi, struct erofs_map_dev *map);
+int erofs_get_occupied_size(const struct erofs_inode *inode,
+			    erofs_off_t *size);
 /* zmap.c */
 int z_erofs_fill_inode(struct erofs_inode *vi);
 int z_erofs_map_blocks_iter(struct erofs_inode *vi,
diff --git a/lib/data.c b/lib/data.c
index 27710f9..92e54b5 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -320,3 +320,24 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
 	}
 	return -EINVAL;
 }
+
+int erofs_get_occupied_size(const struct erofs_inode *inode,
+			    erofs_off_t *size)
+{
+	*size = 0;
+	switch (inode->datalayout) {
+	case EROFS_INODE_FLAT_INLINE:
+	case EROFS_INODE_FLAT_PLAIN:
+	case EROFS_INODE_CHUNK_BASED:
+		*size = inode->i_size;
+		break;
+	case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
+	case EROFS_INODE_FLAT_COMPRESSION:
+		*size = inode->u.i_blocks * EROFS_BLKSIZ;
+		break;
+	default:
+		erofs_err("unknown datalayout");
+		return -1;
+	}
+	return 0;
+}
-- 
2.34.1.307.g9b7440fafd-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 1/2] Add some comments about const-ness around iterate API
  2021-12-21 14:28 [PATCH v1 1/2] Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
  2021-12-21 14:28 ` [PATCH v1 2/2] Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
@ 2021-12-22  1:44 ` Gao Xiang
  2021-12-22  1:49   ` [PATCH v2 1/3] erofs-utils: lib: " Kelvin Zhang via Linux-erofs
  1 sibling, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2021-12-22  1:44 UTC (permalink / raw)
  To: Kelvin Zhang; +Cc: Miao Xie, linux-erofs mailing list

Hi Kelvin,

On Tue, Dec 21, 2021 at 06:28:28AM -0800, Kelvin Zhang wrote:
> Change-Id: I297a56ba14a37ef5eced95330a5b09109378ca44
> Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>

Would you mind add "erofs-utils: lib: " to the subject prefix,
also drop the "Change-Id:" field so I could apply it.

Adding some commit message is better, even it's trivial and
simple.

Thanks,
Gao Xiang

> ---
>  include/erofs/dir.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/include/erofs/dir.h b/include/erofs/dir.h
> index 77656ca..59bd40d 100644
> --- a/include/erofs/dir.h
> +++ b/include/erofs/dir.h
> @@ -39,6 +39,14 @@ typedef int (*erofs_readdir_cb)(struct erofs_dir_context *);
>   * the callback context. |de_namelen| is the exact dirent name length.
>   */
>  struct erofs_dir_context {
> +	/* During execution of |erofs_iterate_dir|, the function needs
> +	 * to read the values inside |erofs_inode* dir|. So it is important
> +	 * that the callback function does not modify stuct pointed by
> +	 * |dir|. It is OK to repoint |dir| to other objects.
> +	 * Unfortunately, it's not possible to enforce this restriction
> +	 * with const keyword, as |erofs_iterate_dir| needs to modify
> +	 * struct pointed by |dir|.
> +	 */
>  	struct erofs_inode *dir;
>  	erofs_readdir_cb cb;
>  	erofs_nid_t pnid;		/* optional */
> -- 
> 2.34.1.307.g9b7440fafd-goog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] Add API to get on disk size of an inode
  2021-12-21 14:28 ` [PATCH v1 2/2] Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
@ 2021-12-22  1:46   ` Gao Xiang
  2021-12-22  1:46     ` Kelvin Zhang via Linux-erofs
  0 siblings, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2021-12-22  1:46 UTC (permalink / raw)
  To: Kelvin Zhang; +Cc: Miao Xie, linux-erofs mailing list

On Tue, Dec 21, 2021 at 06:28:29AM -0800, Kelvin Zhang wrote:
> Change-Id: I60fa9346737b14418bd3fa1d12f760aaf0a0cca5
> Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>

The same to the previous patch.

Also could we remove dump_get_occupied_size() entirely?

Thanks,
Gao Xiang

> ---
>  dump/main.c              |  4 ++--
>  include/erofs/internal.h |  2 ++
>  lib/data.c               | 21 +++++++++++++++++++++
>  3 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/dump/main.c b/dump/main.c
> index 71b44b4..cdde561 100644
> --- a/dump/main.c
> +++ b/dump/main.c
> @@ -175,7 +175,7 @@ static int erofsdump_parse_options_cfg(int argc, char **argv)
>  	return 0;
>  }
>  
> -static int erofs_get_occupied_size(struct erofs_inode *inode,
> +static int dump_get_occupied_size(struct erofs_inode *inode,
>  		erofs_off_t *size)
>  {
>  	*size = 0;
> @@ -291,7 +291,7 @@ static int erofs_read_dirent(struct erofs_dirent *de,
>  		return err;
>  	}
>  
> -	err = erofs_get_occupied_size(&inode, &occupied_size);
> +	err = dump_get_occupied_size(&inode, &occupied_size);
>  	if (err) {
>  		erofs_err("get file size failed\n");
>  		return err;
> diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> index 947304f..8f13e69 100644
> --- a/include/erofs/internal.h
> +++ b/include/erofs/internal.h
> @@ -320,6 +320,8 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
>  int erofs_map_blocks(struct erofs_inode *inode,
>  		struct erofs_map_blocks *map, int flags);
>  int erofs_map_dev(struct erofs_sb_info *sbi, struct erofs_map_dev *map);
> +int erofs_get_occupied_size(const struct erofs_inode *inode,
> +			    erofs_off_t *size);
>  /* zmap.c */
>  int z_erofs_fill_inode(struct erofs_inode *vi);
>  int z_erofs_map_blocks_iter(struct erofs_inode *vi,
> diff --git a/lib/data.c b/lib/data.c
> index 27710f9..92e54b5 100644
> --- a/lib/data.c
> +++ b/lib/data.c
> @@ -320,3 +320,24 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
>  	}
>  	return -EINVAL;
>  }
> +
> +int erofs_get_occupied_size(const struct erofs_inode *inode,
> +			    erofs_off_t *size)
> +{
> +	*size = 0;
> +	switch (inode->datalayout) {
> +	case EROFS_INODE_FLAT_INLINE:
> +	case EROFS_INODE_FLAT_PLAIN:
> +	case EROFS_INODE_CHUNK_BASED:
> +		*size = inode->i_size;
> +		break;
> +	case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
> +	case EROFS_INODE_FLAT_COMPRESSION:
> +		*size = inode->u.i_blocks * EROFS_BLKSIZ;
> +		break;
> +	default:
> +		erofs_err("unknown datalayout");
> +		return -1;
> +	}
> +	return 0;
> +}
> -- 
> 2.34.1.307.g9b7440fafd-goog

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] Add API to get on disk size of an inode
  2021-12-22  1:46   ` Gao Xiang
@ 2021-12-22  1:46     ` Kelvin Zhang via Linux-erofs
  2021-12-22  1:50       ` Gao Xiang
  0 siblings, 1 reply; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2021-12-22  1:46 UTC (permalink / raw)
  To: Gao Xiang; +Cc: Miao Xie, linux-erofs mailing list

Not really, dump_get_occupied_size() contains some custom logic for
counting stats.

On Tue, Dec 21, 2021 at 8:46 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> On Tue, Dec 21, 2021 at 06:28:29AM -0800, Kelvin Zhang wrote:
> > Change-Id: I60fa9346737b14418bd3fa1d12f760aaf0a0cca5
> > Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
>
> The same to the previous patch.
>
> Also could we remove dump_get_occupied_size() entirely?
>
> Thanks,
> Gao Xiang
>
> > ---
> >  dump/main.c              |  4 ++--
> >  include/erofs/internal.h |  2 ++
> >  lib/data.c               | 21 +++++++++++++++++++++
> >  3 files changed, 25 insertions(+), 2 deletions(-)
> >
> > diff --git a/dump/main.c b/dump/main.c
> > index 71b44b4..cdde561 100644
> > --- a/dump/main.c
> > +++ b/dump/main.c
> > @@ -175,7 +175,7 @@ static int erofsdump_parse_options_cfg(int argc, char **argv)
> >       return 0;
> >  }
> >
> > -static int erofs_get_occupied_size(struct erofs_inode *inode,
> > +static int dump_get_occupied_size(struct erofs_inode *inode,
> >               erofs_off_t *size)
> >  {
> >       *size = 0;
> > @@ -291,7 +291,7 @@ static int erofs_read_dirent(struct erofs_dirent *de,
> >               return err;
> >       }
> >
> > -     err = erofs_get_occupied_size(&inode, &occupied_size);
> > +     err = dump_get_occupied_size(&inode, &occupied_size);
> >       if (err) {
> >               erofs_err("get file size failed\n");
> >               return err;
> > diff --git a/include/erofs/internal.h b/include/erofs/internal.h
> > index 947304f..8f13e69 100644
> > --- a/include/erofs/internal.h
> > +++ b/include/erofs/internal.h
> > @@ -320,6 +320,8 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
> >  int erofs_map_blocks(struct erofs_inode *inode,
> >               struct erofs_map_blocks *map, int flags);
> >  int erofs_map_dev(struct erofs_sb_info *sbi, struct erofs_map_dev *map);
> > +int erofs_get_occupied_size(const struct erofs_inode *inode,
> > +                         erofs_off_t *size);
> >  /* zmap.c */
> >  int z_erofs_fill_inode(struct erofs_inode *vi);
> >  int z_erofs_map_blocks_iter(struct erofs_inode *vi,
> > diff --git a/lib/data.c b/lib/data.c
> > index 27710f9..92e54b5 100644
> > --- a/lib/data.c
> > +++ b/lib/data.c
> > @@ -320,3 +320,24 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
> >       }
> >       return -EINVAL;
> >  }
> > +
> > +int erofs_get_occupied_size(const struct erofs_inode *inode,
> > +                         erofs_off_t *size)
> > +{
> > +     *size = 0;
> > +     switch (inode->datalayout) {
> > +     case EROFS_INODE_FLAT_INLINE:
> > +     case EROFS_INODE_FLAT_PLAIN:
> > +     case EROFS_INODE_CHUNK_BASED:
> > +             *size = inode->i_size;
> > +             break;
> > +     case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
> > +     case EROFS_INODE_FLAT_COMPRESSION:
> > +             *size = inode->u.i_blocks * EROFS_BLKSIZ;
> > +             break;
> > +     default:
> > +             erofs_err("unknown datalayout");
> > +             return -1;
> > +     }
> > +     return 0;
> > +}
> > --
> > 2.34.1.307.g9b7440fafd-goog



-- 
Sincerely,

Kelvin Zhang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API
  2021-12-22  1:44 ` [PATCH v1 1/2] Add some comments about const-ness around iterate API Gao Xiang
@ 2021-12-22  1:49   ` Kelvin Zhang via Linux-erofs
  2021-12-22  1:49     ` [PATCH v2 2/3] erofs-utils: lib: Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
  2022-01-04 23:37     ` [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
  0 siblings, 2 replies; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2021-12-22  1:49 UTC (permalink / raw)
  To: linux-erofs mailing list, Miao Xie, Fang Wei; +Cc: Kelvin Zhang

The new iterate dir API has non-trivial const correctness requirements.
Document them in comment.

Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
---
 include/erofs/dir.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/erofs/dir.h b/include/erofs/dir.h
index 77656ca..59bd40d 100644
--- a/include/erofs/dir.h
+++ b/include/erofs/dir.h
@@ -39,6 +39,14 @@ typedef int (*erofs_readdir_cb)(struct erofs_dir_context *);
  * the callback context. |de_namelen| is the exact dirent name length.
  */
 struct erofs_dir_context {
+	/* During execution of |erofs_iterate_dir|, the function needs
+	 * to read the values inside |erofs_inode* dir|. So it is important
+	 * that the callback function does not modify stuct pointed by
+	 * |dir|. It is OK to repoint |dir| to other objects.
+	 * Unfortunately, it's not possible to enforce this restriction
+	 * with const keyword, as |erofs_iterate_dir| needs to modify
+	 * struct pointed by |dir|.
+	 */
 	struct erofs_inode *dir;
 	erofs_readdir_cb cb;
 	erofs_nid_t pnid;		/* optional */
-- 
2.34.1.448.ga2b2bfdf31-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 2/3] erofs-utils: lib: Add API to get on disk size of an inode
  2021-12-22  1:49   ` [PATCH v2 1/3] erofs-utils: lib: " Kelvin Zhang via Linux-erofs
@ 2021-12-22  1:49     ` Kelvin Zhang via Linux-erofs
  2022-01-04 23:37     ` [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
  1 sibling, 0 replies; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2021-12-22  1:49 UTC (permalink / raw)
  To: linux-erofs mailing list, Miao Xie, Fang Wei; +Cc: Kelvin Zhang

Marginally improve code re-use. It's quite common for users to query for
compressed size of an inode.

Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
---
 dump/main.c              |  4 ++--
 include/erofs/internal.h |  2 ++
 lib/data.c               | 21 +++++++++++++++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/dump/main.c b/dump/main.c
index 71b44b4..cdde561 100644
--- a/dump/main.c
+++ b/dump/main.c
@@ -175,7 +175,7 @@ static int erofsdump_parse_options_cfg(int argc, char **argv)
 	return 0;
 }
 
-static int erofs_get_occupied_size(struct erofs_inode *inode,
+static int dump_get_occupied_size(struct erofs_inode *inode,
 		erofs_off_t *size)
 {
 	*size = 0;
@@ -291,7 +291,7 @@ static int erofs_read_dirent(struct erofs_dirent *de,
 		return err;
 	}
 
-	err = erofs_get_occupied_size(&inode, &occupied_size);
+	err = dump_get_occupied_size(&inode, &occupied_size);
 	if (err) {
 		erofs_err("get file size failed\n");
 		return err;
diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index 947304f..8f13e69 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -320,6 +320,8 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
 int erofs_map_blocks(struct erofs_inode *inode,
 		struct erofs_map_blocks *map, int flags);
 int erofs_map_dev(struct erofs_sb_info *sbi, struct erofs_map_dev *map);
+int erofs_get_occupied_size(const struct erofs_inode *inode,
+			    erofs_off_t *size);
 /* zmap.c */
 int z_erofs_fill_inode(struct erofs_inode *vi);
 int z_erofs_map_blocks_iter(struct erofs_inode *vi,
diff --git a/lib/data.c b/lib/data.c
index 27710f9..92e54b5 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -320,3 +320,24 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
 	}
 	return -EINVAL;
 }
+
+int erofs_get_occupied_size(const struct erofs_inode *inode,
+			    erofs_off_t *size)
+{
+	*size = 0;
+	switch (inode->datalayout) {
+	case EROFS_INODE_FLAT_INLINE:
+	case EROFS_INODE_FLAT_PLAIN:
+	case EROFS_INODE_CHUNK_BASED:
+		*size = inode->i_size;
+		break;
+	case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
+	case EROFS_INODE_FLAT_COMPRESSION:
+		*size = inode->u.i_blocks * EROFS_BLKSIZ;
+		break;
+	default:
+		erofs_err("unknown datalayout");
+		return -1;
+	}
+	return 0;
+}
-- 
2.34.1.448.ga2b2bfdf31-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v1 2/2] Add API to get on disk size of an inode
  2021-12-22  1:46     ` Kelvin Zhang via Linux-erofs
@ 2021-12-22  1:50       ` Gao Xiang
  2021-12-22  1:54         ` [PATCH v3 1/2] erofs-utils: lib: Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
  0 siblings, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2021-12-22  1:50 UTC (permalink / raw)
  To: Kelvin Zhang; +Cc: Miao Xie, linux-erofs mailing list

On Tue, Dec 21, 2021 at 08:46:58PM -0500, Kelvin Zhang wrote:
> Not really, dump_get_occupied_size() contains some custom logic for
> counting stats.
> 

Ok, thanks. maybe it would be better to rename as
erofsdump_get_occupied_size().
Also we could "static inline" such API since it's simple.

Thanks,
Gao Xiang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 1/2] erofs-utils: lib: Add some comments about const-ness around iterate API
  2021-12-22  1:50       ` Gao Xiang
@ 2021-12-22  1:54         ` Kelvin Zhang via Linux-erofs
  2021-12-22  1:54           ` [PATCH v3 2/2] erofs-utils: lib: Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
  0 siblings, 1 reply; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2021-12-22  1:54 UTC (permalink / raw)
  To: linux-erofs mailing list, Miao Xie, Fang Wei; +Cc: Kelvin Zhang

The new iterate dir API has non-trivial const correctness requirements.
Document them in comment.

Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
---
 include/erofs/dir.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/erofs/dir.h b/include/erofs/dir.h
index 77656ca..59bd40d 100644
--- a/include/erofs/dir.h
+++ b/include/erofs/dir.h
@@ -39,6 +39,14 @@ typedef int (*erofs_readdir_cb)(struct erofs_dir_context *);
  * the callback context. |de_namelen| is the exact dirent name length.
  */
 struct erofs_dir_context {
+	/* During execution of |erofs_iterate_dir|, the function needs
+	 * to read the values inside |erofs_inode* dir|. So it is important
+	 * that the callback function does not modify stuct pointed by
+	 * |dir|. It is OK to repoint |dir| to other objects.
+	 * Unfortunately, it's not possible to enforce this restriction
+	 * with const keyword, as |erofs_iterate_dir| needs to modify
+	 * struct pointed by |dir|.
+	 */
 	struct erofs_inode *dir;
 	erofs_readdir_cb cb;
 	erofs_nid_t pnid;		/* optional */
-- 
2.34.1.448.ga2b2bfdf31-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v3 2/2] erofs-utils: lib: Add API to get on disk size of an inode
  2021-12-22  1:54         ` [PATCH v3 1/2] erofs-utils: lib: Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
@ 2021-12-22  1:54           ` Kelvin Zhang via Linux-erofs
  0 siblings, 0 replies; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2021-12-22  1:54 UTC (permalink / raw)
  To: linux-erofs mailing list, Miao Xie, Fang Wei; +Cc: Kelvin Zhang

Marginally improve code re-use. It's quite common for users to query for
compressed size of an inode.

Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
---
 dump/main.c              |  4 ++--
 include/erofs/internal.h | 23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/dump/main.c b/dump/main.c
index 71b44b4..4c2c513 100644
--- a/dump/main.c
+++ b/dump/main.c
@@ -175,7 +175,7 @@ static int erofsdump_parse_options_cfg(int argc, char **argv)
 	return 0;
 }
 
-static int erofs_get_occupied_size(struct erofs_inode *inode,
+static int erofsdump_get_occupied_size(struct erofs_inode *inode,
 		erofs_off_t *size)
 {
 	*size = 0;
@@ -291,7 +291,7 @@ static int erofs_read_dirent(struct erofs_dirent *de,
 		return err;
 	}
 
-	err = erofs_get_occupied_size(&inode, &occupied_size);
+	err = erofsdump_get_occupied_size(&inode, &occupied_size);
 	if (err) {
 		erofs_err("get file size failed\n");
 		return err;
diff --git a/include/erofs/internal.h b/include/erofs/internal.h
index 947304f..c64cf36 100644
--- a/include/erofs/internal.h
+++ b/include/erofs/internal.h
@@ -19,6 +19,7 @@ typedef unsigned short umode_t;
 
 #define __packed __attribute__((__packed__))
 
+#include "erofs/print.h"
 #include "erofs_fs.h"
 #include <fcntl.h>
 
@@ -320,6 +321,28 @@ int erofs_pread(struct erofs_inode *inode, char *buf,
 int erofs_map_blocks(struct erofs_inode *inode,
 		struct erofs_map_blocks *map, int flags);
 int erofs_map_dev(struct erofs_sb_info *sbi, struct erofs_map_dev *map);
+
+static inline int erofs_get_occupied_size(const struct erofs_inode *inode,
+					  erofs_off_t *size)
+{
+	*size = 0;
+	switch (inode->datalayout) {
+	case EROFS_INODE_FLAT_INLINE:
+	case EROFS_INODE_FLAT_PLAIN:
+	case EROFS_INODE_CHUNK_BASED:
+		*size = inode->i_size;
+		break;
+	case EROFS_INODE_FLAT_COMPRESSION_LEGACY:
+	case EROFS_INODE_FLAT_COMPRESSION:
+		*size = inode->u.i_blocks * EROFS_BLKSIZ;
+		break;
+	default:
+		erofs_err("unknown datalayout");
+		return -1;
+	}
+	return 0;
+}
+
 /* zmap.c */
 int z_erofs_fill_inode(struct erofs_inode *vi);
 int z_erofs_map_blocks_iter(struct erofs_inode *vi,
-- 
2.34.1.448.ga2b2bfdf31-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API
  2021-12-22  1:49   ` [PATCH v2 1/3] erofs-utils: lib: " Kelvin Zhang via Linux-erofs
  2021-12-22  1:49     ` [PATCH v2 2/3] erofs-utils: lib: Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
@ 2022-01-04 23:37     ` Kelvin Zhang via Linux-erofs
  2022-01-05  0:23       ` Gao Xiang
  2022-01-05  0:51       ` Gao Xiang
  1 sibling, 2 replies; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2022-01-04 23:37 UTC (permalink / raw)
  To: linux-erofs mailing list, Miao Xie, Fang Wei

friendly ping

On Tue, Dec 21, 2021 at 5:49 PM Kelvin Zhang <zhangkelvin@google.com> wrote:
>
> The new iterate dir API has non-trivial const correctness requirements.
> Document them in comment.
>
> Signed-off-by: Kelvin Zhang <zhangkelvin@google.com>
> ---
>  include/erofs/dir.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/include/erofs/dir.h b/include/erofs/dir.h
> index 77656ca..59bd40d 100644
> --- a/include/erofs/dir.h
> +++ b/include/erofs/dir.h
> @@ -39,6 +39,14 @@ typedef int (*erofs_readdir_cb)(struct erofs_dir_context *);
>   * the callback context. |de_namelen| is the exact dirent name length.
>   */
>  struct erofs_dir_context {
> +       /* During execution of |erofs_iterate_dir|, the function needs
> +        * to read the values inside |erofs_inode* dir|. So it is important
> +        * that the callback function does not modify stuct pointed by
> +        * |dir|. It is OK to repoint |dir| to other objects.
> +        * Unfortunately, it's not possible to enforce this restriction
> +        * with const keyword, as |erofs_iterate_dir| needs to modify
> +        * struct pointed by |dir|.
> +        */
>         struct erofs_inode *dir;
>         erofs_readdir_cb cb;
>         erofs_nid_t pnid;               /* optional */
> --
> 2.34.1.448.ga2b2bfdf31-goog
>


-- 
Sincerely,

Kelvin Zhang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API
  2022-01-04 23:37     ` [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
@ 2022-01-05  0:23       ` Gao Xiang
  2022-01-05  0:24         ` Kelvin Zhang via Linux-erofs
  2022-01-05  0:51       ` Gao Xiang
  1 sibling, 1 reply; 14+ messages in thread
From: Gao Xiang @ 2022-01-05  0:23 UTC (permalink / raw)
  To: Kelvin Zhang; +Cc: Miao Xie, linux-erofs mailing list

On Tue, Jan 04, 2022 at 03:37:51PM -0800, Kelvin Zhang wrote:
> friendly ping

I already merged them, didn't I?
https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=dev&id=2be280dc28ace9c3840aa5e6ca7ff90ef4212cd1

Thanks,
Gao Xiang

> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API
  2022-01-05  0:23       ` Gao Xiang
@ 2022-01-05  0:24         ` Kelvin Zhang via Linux-erofs
  0 siblings, 0 replies; 14+ messages in thread
From: Kelvin Zhang via Linux-erofs @ 2022-01-05  0:24 UTC (permalink / raw)
  To: Gao Xiang; +Cc: Miao Xie, linux-erofs mailing list

Ah, I see, thanks!

On Tue, Jan 4, 2022 at 4:23 PM Gao Xiang <hsiangkao@linux.alibaba.com> wrote:
>
> On Tue, Jan 04, 2022 at 03:37:51PM -0800, Kelvin Zhang wrote:
> > friendly ping
>
> I already merged them, didn't I?
> https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=dev&id=2be280dc28ace9c3840aa5e6ca7ff90ef4212cd1
>
> Thanks,
> Gao Xiang
>
> >



-- 
Sincerely,

Kelvin Zhang

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API
  2022-01-04 23:37     ` [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
  2022-01-05  0:23       ` Gao Xiang
@ 2022-01-05  0:51       ` Gao Xiang
  1 sibling, 0 replies; 14+ messages in thread
From: Gao Xiang @ 2022-01-05  0:51 UTC (permalink / raw)
  To: Kelvin Zhang; +Cc: Miao Xie, linux-erofs mailing list

On Tue, Jan 04, 2022 at 03:37:51PM -0800, Kelvin Zhang wrote:
> friendly ping

I already merged them. Didn't I?
https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/commit/?h=dev&id=2be280dc28ace9c3840aa5e6ca7ff90ef4212cd1

Thanks,
Gao Xiang

> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-01-05  0:53 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-21 14:28 [PATCH v1 1/2] Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
2021-12-21 14:28 ` [PATCH v1 2/2] Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
2021-12-22  1:46   ` Gao Xiang
2021-12-22  1:46     ` Kelvin Zhang via Linux-erofs
2021-12-22  1:50       ` Gao Xiang
2021-12-22  1:54         ` [PATCH v3 1/2] erofs-utils: lib: Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
2021-12-22  1:54           ` [PATCH v3 2/2] erofs-utils: lib: Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
2021-12-22  1:44 ` [PATCH v1 1/2] Add some comments about const-ness around iterate API Gao Xiang
2021-12-22  1:49   ` [PATCH v2 1/3] erofs-utils: lib: " Kelvin Zhang via Linux-erofs
2021-12-22  1:49     ` [PATCH v2 2/3] erofs-utils: lib: Add API to get on disk size of an inode Kelvin Zhang via Linux-erofs
2022-01-04 23:37     ` [PATCH v2 1/3] erofs-utils: lib: Add some comments about const-ness around iterate API Kelvin Zhang via Linux-erofs
2022-01-05  0:23       ` Gao Xiang
2022-01-05  0:24         ` Kelvin Zhang via Linux-erofs
2022-01-05  0:51       ` Gao Xiang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.