[PATCH 0/2] minix: modernize function declarations and eliminating some magic numbers

[PATCH 0/2] minix: modernize function declarations and eliminating some magic numbers

[Jeremy Bingham]

Another set of minix patches fixing issues that I noticed while working
on other things. The various dirsize and namelen values being set in
minix_fill_super() in inode.c, which vary depending on the versions of
the minix filesystem and how it was created, were being set as magic
numbers. The function declarations in minix.h were also a mix of current
and historic kernel coding standards, where some function declarations
had arguments with types and names while others had only types. This
became an annoyance when updating minix.h because checkpatch.pl would
gripe about improper function declarations.

Create and use #defines for the various dirsize and namelen values in
minix_fill_super, and modernize the function declarations in minix.h.

Both patches have been tested by mounting and performing file operations
against v1, v2, and v3 minix filesystem volumes. Neither of them have
any functional changes.

Jeremy Bingham (2):
  minix: define constants for dirsize and namelen
  minix: modernize function declarations in minix.h

 fs/minix/inode.c    | 20 ++++++------
 fs/minix/itree_v1.c |  2 +-
 fs/minix/itree_v2.c |  2 +-
 fs/minix/minix.h    | 78 ++++++++++++++++++++++++++++++---------------
 4 files changed, 65 insertions(+), 37 deletions(-)

-- 
2.47.3

[PATCH 1/2] minix: define constants for dirsize and namelen

[Jeremy Bingham]

The different versions of the minix fs have different values for
directory size and name length. Versions 1 and 2 each have two possible
values for each of those, while version 3 has one set of values for
each. Add defines for all of these to minix.h so we don't need to set
those values as magic numbers in minix_fill_super() in inode.c

Before:
	sbi->s_dirsize = 16;
After:
	sbi->s_dirsize = MINIX_V1_DIRSIZE;

There are no functional changes with this patch.

Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
 fs/minix/inode.c | 20 ++++++++++----------
 fs/minix/minix.h | 23 ++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 11 deletions(-)

diff --git a/fs/minix/inode.c b/fs/minix/inode.c
index c30cc590698d..95fa8371e4ea 100644
--- a/fs/minix/inode.c
+++ b/fs/minix/inode.c
@@ -258,25 +258,25 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_magic = ms->s_magic;
 	if (s->s_magic == MINIX_SUPER_MAGIC) {
 		sbi->s_version = MINIX_V1;
-		sbi->s_dirsize = 16;
-		sbi->s_namelen = 14;
+		sbi->s_dirsize = MINIX_V1_DIRSIZE;
+		sbi->s_namelen = MINIX_V1_NAMELEN;
 		s->s_max_links = MINIX_LINK_MAX;
 	} else if (s->s_magic == MINIX_SUPER_MAGIC2) {
 		sbi->s_version = MINIX_V1;
-		sbi->s_dirsize = 32;
-		sbi->s_namelen = 30;
+		sbi->s_dirsize = MINIX_V1_DIRSIZE2;
+		sbi->s_namelen = MINIX_V1_NAMELEN2;
 		s->s_max_links = MINIX_LINK_MAX;
 	} else if (s->s_magic == MINIX2_SUPER_MAGIC) {
 		sbi->s_version = MINIX_V2;
 		sbi->s_nzones = ms->s_zones;
-		sbi->s_dirsize = 16;
-		sbi->s_namelen = 14;
+		sbi->s_dirsize = MINIX_V2_DIRSIZE;
+		sbi->s_namelen = MINIX_V2_NAMELEN;
 		s->s_max_links = MINIX2_LINK_MAX;
 	} else if (s->s_magic == MINIX2_SUPER_MAGIC2) {
 		sbi->s_version = MINIX_V2;
 		sbi->s_nzones = ms->s_zones;
-		sbi->s_dirsize = 32;
-		sbi->s_namelen = 30;
+		sbi->s_dirsize = MINIX_V2_DIRSIZE2;
+		sbi->s_namelen = MINIX_V2_NAMELEN2;
 		s->s_max_links = MINIX2_LINK_MAX;
 	} else if ( *(__u16 *)(bh->b_data + 24) == MINIX3_SUPER_MAGIC) {
 		m3s = (struct minix3_super_block *) bh->b_data;
@@ -288,8 +288,8 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
 		s->s_maxbytes = m3s->s_max_size;
 		sbi->s_ninodes = m3s->s_ninodes;
 		sbi->s_nzones = m3s->s_zones;
-		sbi->s_dirsize = 64;
-		sbi->s_namelen = 60;
+		sbi->s_dirsize = MINIX_V3_DIRSIZE;
+		sbi->s_namelen = MINIX_V3_NAMELEN;
 		sbi->s_version = MINIX_V3;
 		sbi->s_mount_state = MINIX_VALID_FS;
 		if (!sb_set_blocksize(s, m3s->s_blocksize))
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index f2025c9b5825..972a71c5b481 100644
--- a/fs/minix/minix.h
+++ b/fs/minix/minix.h
@@ -11,6 +11,27 @@
 #define MINIX_V2		0x0002		/* minix V2 fs */
 #define MINIX_V3		0x0003		/* minix V3 fs */
 
+/* Define version and sub-version specific values for dirsize and name length.
+ * These are not as clear-cut as having different values for versions 1, 2, and
+ * 3, unfortunately, but rather there two possible values for each for versions
+ * 1 and 2 and one possible set of values for version 3.
+ *
+ * These have separate defines for each subversion value for versions 1 and 2,
+ * though; even though they duplicate each other, it will be more clear to have
+ * them separate than trying to come up with a name that encompasses both
+ * versions yet not be totally confusing.
+ */
+#define MINIX_V1_DIRSIZE	16
+#define MINIX_V1_NAMELEN	14
+#define MINIX_V1_DIRSIZE2	32
+#define MINIX_V1_NAMELEN2	30
+#define MINIX_V2_DIRSIZE	16
+#define MINIX_V2_NAMELEN	14
+#define MINIX_V2_DIRSIZE2	32
+#define MINIX_V2_NAMELEN2	30
+#define MINIX_V3_DIRSIZE	64
+#define MINIX_V3_NAMELEN	60
+
 /*
  * minix fs inode data in memory
  */
-- 
2.47.3

[PATCH 2/2] minix: modernize function declarations in minix.h

[Jeremy Bingham]

The function prototypes in minix.h were a mix of current and historic
standards. Some functions had arguments with types and names while
others had just the types. Some functions were also still prefixed with
extern, and there were a few uses of bare 'unsigned' still as well.

This brings fs/minix/minix.h up to speed with the current formatting
standards. Function declarations are consistent with types and names
both declared, and the bare 'unsigned' variables are now declared as
'unsigned int'.

There are no functional changes with this patch.

Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
---
 fs/minix/itree_v1.c |  2 +-
 fs/minix/itree_v2.c |  2 +-
 fs/minix/minix.h    | 55 +++++++++++++++++++++++++--------------------
 3 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/fs/minix/itree_v1.c b/fs/minix/itree_v1.c
index 1fed906042aa..3433f50e2de2 100644
--- a/fs/minix/itree_v1.c
+++ b/fs/minix/itree_v1.c
@@ -61,7 +61,7 @@ void V1_minix_truncate(struct inode * inode)
 	truncate(inode);
 }
 
-unsigned V1_minix_blocks(loff_t size, struct super_block *sb)
+unsigned int V1_minix_blocks(loff_t size, struct super_block *sb)
 {
 	return nblocks(size, sb);
 }
diff --git a/fs/minix/itree_v2.c b/fs/minix/itree_v2.c
index 9d00f31a2d9d..a18b6e1f9ed5 100644
--- a/fs/minix/itree_v2.c
+++ b/fs/minix/itree_v2.c
@@ -69,7 +69,7 @@ void V2_minix_truncate(struct inode * inode)
 	truncate(inode);
 }
 
-unsigned V2_minix_blocks(loff_t size, struct super_block *sb)
+unsigned int V2_minix_blocks(loff_t size, struct super_block *sb)
 {
 	return nblocks(size, sb);
 }
diff --git a/fs/minix/minix.h b/fs/minix/minix.h
index 972a71c5b481..d124145850e9 100644
--- a/fs/minix/minix.h
+++ b/fs/minix/minix.h
@@ -67,39 +67,44 @@ struct minix_sb_info {
 void __minix_error_inode(struct inode *inode, const char *function,
 			 unsigned int line, const char *fmt, ...);
 
-struct inode *minix_iget(struct super_block *, unsigned long);
-struct minix_inode *minix_V1_raw_inode(struct super_block *, ino_t, struct buffer_head **);
-struct minix2_inode *minix_V2_raw_inode(struct super_block *, ino_t, struct buffer_head **);
-struct inode *minix_new_inode(const struct inode *, umode_t);
+struct inode *minix_iget(struct super_block *sb, unsigned long ino);
+struct minix_inode *minix_V1_raw_inode(struct super_block *sb, ino_t ino,
+		struct buffer_head **bh);
+struct minix2_inode *minix_V2_raw_inode(struct super_block *sb, ino_t ino,
+		struct buffer_head **bh);
+struct inode *minix_new_inode(const struct inode *dir, umode_t mode);
 void minix_free_inode(struct inode *inode);
 unsigned long minix_count_free_inodes(struct super_block *sb);
 int minix_new_block(struct inode *inode);
 void minix_free_block(struct inode *inode, unsigned long block);
 unsigned long minix_count_free_blocks(struct super_block *sb);
-int minix_getattr(struct mnt_idmap *, const struct path *,
-		struct kstat *, u32, unsigned int);
+int minix_getattr(struct mnt_idmap *idmap, const struct path *path,
+		struct kstat *stat, u32 request_mask, unsigned int flags);
 int minix_prepare_chunk(struct folio *folio, loff_t pos, unsigned len);
 struct mapping_metadata_bhs *minix_get_metadata_bhs(struct inode *inode);
 int minix_fsync(struct file *file, loff_t start, loff_t end, int datasync);
 
-extern void V1_minix_truncate(struct inode *);
-extern void V2_minix_truncate(struct inode *);
-extern void minix_truncate(struct inode *);
-extern void minix_set_inode(struct inode *, dev_t);
-extern int V1_minix_get_block(struct inode *, long, struct buffer_head *, int);
-extern int V2_minix_get_block(struct inode *, long, struct buffer_head *, int);
-extern unsigned V1_minix_blocks(loff_t, struct super_block *);
-extern unsigned V2_minix_blocks(loff_t, struct super_block *);
-
-struct minix_dir_entry *minix_find_entry(struct dentry *, struct folio **);
-int minix_add_link(struct dentry*, struct inode*);
-int minix_delete_entry(struct minix_dir_entry *, struct folio *);
-int minix_make_empty(struct inode*, struct inode*);
-int minix_empty_dir(struct inode*);
+void V1_minix_truncate(struct inode *inode);
+void V2_minix_truncate(struct inode *inode);
+void minix_truncate(struct inode *inode);
+void minix_set_inode(struct inode *inode, dev_t rdev);
+int V1_minix_get_block(struct inode *inode, long block,
+		struct buffer_head *bh, int create);
+int V2_minix_get_block(struct inode *inode, long block,
+		struct buffer_head *bh, int create);
+unsigned int V1_minix_blocks(loff_t size, struct super_block *sb);
+unsigned int V2_minix_blocks(loff_t size, struct super_block *sb);
+
+struct minix_dir_entry *minix_find_entry(struct dentry *dentry,
+	struct folio **foliop);
+int minix_add_link(struct dentry *dentry, struct inode *inode);
+int minix_delete_entry(struct minix_dir_entry *de, struct folio *folio);
+int minix_make_empty(struct inode *inode, struct inode *dir);
+int minix_empty_dir(struct inode *inode);
 int minix_set_link(struct minix_dir_entry *de, struct folio *folio,
 		struct inode *inode);
-struct minix_dir_entry *minix_dotdot(struct inode*, struct folio **);
-ino_t minix_inode_by_name(struct dentry*);
+struct minix_dir_entry *minix_dotdot(struct inode *dir, struct folio **foliop);
+ino_t minix_inode_by_name(struct dentry *dentry);
 
 extern const struct inode_operations minix_file_inode_operations;
 extern const struct inode_operations minix_dir_inode_operations;
@@ -116,7 +121,8 @@ static inline struct minix_inode_info *minix_i(struct inode *inode)
 	return container_of(inode, struct minix_inode_info, vfs_inode);
 }
 
-static inline unsigned minix_blocks_needed(unsigned bits, unsigned blocksize)
+static inline unsigned int minix_blocks_needed(unsigned int bits,
+		unsigned int blocksize)
 {
 	return DIV_ROUND_UP(bits, blocksize * 8);
 }
@@ -150,7 +156,8 @@ static inline unsigned minix_blocks_needed(unsigned bits, unsigned blocksize)
  * big-endian 16bit indexed bitmaps
  */
 
-static inline int minix_find_first_zero_bit(const void *vaddr, unsigned size)
+static inline int minix_find_first_zero_bit(const void *vaddr,
+		unsigned int size)
 {
 	const unsigned short *p = vaddr, *addr = vaddr;
 	unsigned short num;
-- 
2.47.3

Re: [PATCH 1/2] minix: define constants for dirsize and namelen

[Jori Koolstra]

> Op 06-07-2026 20:22 CEST schreef Jeremy Bingham <jbingham@gmail.com>:
> 
>  
> The different versions of the minix fs have different values for
> directory size and name length. Versions 1 and 2 each have two possible
> values for each of those, while version 3 has one set of values for
> each. Add defines for all of these to minix.h so we don't need to set
> those values as magic numbers in minix_fill_super() in inode.c
> 
> Before:
> 	sbi->s_dirsize = 16;
> After:
> 	sbi->s_dirsize = MINIX_V1_DIRSIZE;
> 
> There are no functional changes with this patch.
> 
> Signed-off-by: Jeremy Bingham <jbingham@gmail.com>
> ---
>  fs/minix/inode.c | 20 ++++++++++----------
>  fs/minix/minix.h | 23 ++++++++++++++++++++++-
>  2 files changed, 32 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/minix/inode.c b/fs/minix/inode.c
> index c30cc590698d..95fa8371e4ea 100644
> --- a/fs/minix/inode.c
> +++ b/fs/minix/inode.c
> @@ -258,25 +258,25 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
>  	s->s_magic = ms->s_magic;
>  	if (s->s_magic == MINIX_SUPER_MAGIC) {
>  		sbi->s_version = MINIX_V1;
> -		sbi->s_dirsize = 16;
> -		sbi->s_namelen = 14;
> +		sbi->s_dirsize = MINIX_V1_DIRSIZE;
> +		sbi->s_namelen = MINIX_V1_NAMELEN;
>  		s->s_max_links = MINIX_LINK_MAX;
>  	} else if (s->s_magic == MINIX_SUPER_MAGIC2) {
>  		sbi->s_version = MINIX_V1;
> -		sbi->s_dirsize = 32;
> -		sbi->s_namelen = 30;
> +		sbi->s_dirsize = MINIX_V1_DIRSIZE2;
> +		sbi->s_namelen = MINIX_V1_NAMELEN2;
>  		s->s_max_links = MINIX_LINK_MAX;
>  	} else if (s->s_magic == MINIX2_SUPER_MAGIC) {
>  		sbi->s_version = MINIX_V2;
>  		sbi->s_nzones = ms->s_zones;
> -		sbi->s_dirsize = 16;
> -		sbi->s_namelen = 14;
> +		sbi->s_dirsize = MINIX_V2_DIRSIZE;
> +		sbi->s_namelen = MINIX_V2_NAMELEN;
>  		s->s_max_links = MINIX2_LINK_MAX;
>  	} else if (s->s_magic == MINIX2_SUPER_MAGIC2) {
>  		sbi->s_version = MINIX_V2;
>  		sbi->s_nzones = ms->s_zones;
> -		sbi->s_dirsize = 32;
> -		sbi->s_namelen = 30;
> +		sbi->s_dirsize = MINIX_V2_DIRSIZE2;
> +		sbi->s_namelen = MINIX_V2_NAMELEN2;
>  		s->s_max_links = MINIX2_LINK_MAX;
>  	} else if ( *(__u16 *)(bh->b_data + 24) == MINIX3_SUPER_MAGIC) {
>  		m3s = (struct minix3_super_block *) bh->b_data;
> @@ -288,8 +288,8 @@ static int minix_fill_super(struct super_block *s, struct fs_context *fc)
>  		s->s_maxbytes = m3s->s_max_size;
>  		sbi->s_ninodes = m3s->s_ninodes;
>  		sbi->s_nzones = m3s->s_zones;
> -		sbi->s_dirsize = 64;
> -		sbi->s_namelen = 60;
> +		sbi->s_dirsize = MINIX_V3_DIRSIZE;
> +		sbi->s_namelen = MINIX_V3_NAMELEN;
>  		sbi->s_version = MINIX_V3;
>  		sbi->s_mount_state = MINIX_VALID_FS;
>  		if (!sb_set_blocksize(s, m3s->s_blocksize))
> diff --git a/fs/minix/minix.h b/fs/minix/minix.h
> index f2025c9b5825..972a71c5b481 100644
> --- a/fs/minix/minix.h
> +++ b/fs/minix/minix.h
> @@ -11,6 +11,27 @@
>  #define MINIX_V2		0x0002		/* minix V2 fs */
>  #define MINIX_V3		0x0003		/* minix V3 fs */
>  
> +/* Define version and sub-version specific values for dirsize and name length.
> + * These are not as clear-cut as having different values for versions 1, 2, and
> + * 3, unfortunately, but rather there two possible values for each for versions
> + * 1 and 2 and one possible set of values for version 3.
> + *
> + * These have separate defines for each subversion value for versions 1 and 2,
> + * though; even though they duplicate each other, it will be more clear to have
> + * them separate than trying to come up with a name that encompasses both
> + * versions yet not be totally confusing.
> + */
> +#define MINIX_V1_DIRSIZE	16
> +#define MINIX_V1_NAMELEN	14
> +#define MINIX_V1_DIRSIZE2	32
> +#define MINIX_V1_NAMELEN2	30
> +#define MINIX_V2_DIRSIZE	16
> +#define MINIX_V2_NAMELEN	14
> +#define MINIX_V2_DIRSIZE2	32
> +#define MINIX_V2_NAMELEN2	30
> +#define MINIX_V3_DIRSIZE	64
> +#define MINIX_V3_NAMELEN	60
> +
>  /*
>   * minix fs inode data in memory
>   */
> -- 
> 2.47.3

Hi Jeremy,

Thanks for taking the time to look at minix. I have a particular fondness of the
filesystem, so I am happy someone takes an interest.

However, I don't like this series. There is no reason for all this churn or the
exorbitant amount of comments. If you can get iomap to work that would be great,
but please don't do this.

Thanks,
Jori.