[PATCH 1/4] befs: dump inode_size superblock information

[PATCH 1/4] befs: dump inode_size superblock information

[Luis de Bethencourt]

befs_dump_super_block() wasn't giving the inode_size information when
dumping all elements of the superblock. Add this element to have complete
information of the superblock.

Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
---

Hi,

This is a series of superblock information related changes.

Thanks,
Luis

 fs/befs/debug.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/befs/debug.c b/fs/befs/debug.c
index 4de7cff..85c1339 100644
--- a/fs/befs/debug.c
+++ b/fs/befs/debug.c
@@ -169,6 +169,7 @@ befs_dump_super_block(const struct super_block *sb, befs_super_block * sup)
 
 	befs_debug(sb, "  num_blocks %llu", fs64_to_cpu(sb, sup->num_blocks));
 	befs_debug(sb, "  used_blocks %llu", fs64_to_cpu(sb, sup->used_blocks));
+	befs_debug(sb, "  inode_size %u", fs32_to_cpu(sb, sup->inode_size));
 
 	befs_debug(sb, "  magic2 %08x", fs32_to_cpu(sb, sup->magic2));
 	befs_debug(sb, "  blocks_per_ag %u",
-- 
2.5.1

[PATCH 2/4] befs: add check for ag_shift in superblock

[Luis de Bethencourt]

ag_shift and blocks_per_ag contain the same information in different ways,
same as block_shift and block_size do. It is worth checking this two are
consistent, but since blocks_per_ag isn't documented as mandatory to use
some implementations of befs don't enforce this, so making it non-fatal if
they don't match and just having it as a warning.

Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
---
 fs/befs/super.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/fs/befs/super.c b/fs/befs/super.c
index dc13df8..c36745d 100644
--- a/fs/befs/super.c
+++ b/fs/befs/super.c
@@ -103,6 +103,13 @@ befs_check_sb(struct super_block *sb)
 		return BEFS_ERR;
 	}
 
+
+	/* ag_shift also encodes the same information as blocks_per_ag in a
+	 * different way, non-fatal consistency check
+	 */
+	if ((1 << befs_sb->ag_shift) != befs_sb->blocks_per_ag)
+		befs_error(sb, "ag_shift disagrees with blocks_per_ag.");
+
 	if (befs_sb->log_start != befs_sb->log_end || befs_sb->flags == BEFS_DIRTY) {
 		befs_error(sb, "Filesystem not clean! There are blocks in the "
 			   "journal. You must boot into BeOS and mount this volume "
-- 
2.5.1

Re: [PATCH 2/4] befs: add check for ag_shift in superblock

[Salah Triki]

On Tue, Aug 09, 2016 at 11:01:24PM +0100, Luis de Bethencourt wrote:
> ag_shift and blocks_per_ag contain the same information in different ways,
> same as block_shift and block_size do. It is worth checking this two are
> consistent, but since blocks_per_ag isn't documented as mandatory to use
> some implementations of befs don't enforce this, so making it non-fatal if
> they don't match and just having it as a warning.
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
>  fs/befs/super.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/befs/super.c b/fs/befs/super.c
> index dc13df8..c36745d 100644
> --- a/fs/befs/super.c
> +++ b/fs/befs/super.c
> @@ -103,6 +103,13 @@ befs_check_sb(struct super_block *sb)
>  		return BEFS_ERR;
>  	}
>  
> +
> +	/* ag_shift also encodes the same information as blocks_per_ag in a
> +	 * different way, non-fatal consistency check
> +	 */
> +	if ((1 << befs_sb->ag_shift) != befs_sb->blocks_per_ag)
> +		befs_error(sb, "ag_shift disagrees with blocks_per_ag.");
> +
>  	if (befs_sb->log_start != befs_sb->log_end || befs_sb->flags == BEFS_DIRTY) {
>  		befs_error(sb, "Filesystem not clean! There are blocks in the "
>  			   "journal. You must boot into BeOS and mount this volume "
> -- 
> 2.5.1
> 

Signed-off-by: Salah Triki <salah.triki@gmail.com>

Thanx :)
Salah

[PATCH 3/4] befs: fix comment style

[Luis de Bethencourt]

The description of befs_load_sb was confusing the kernel-doc system since,
because it starts with /**, it thinks it will document the function with
kernel-doc formatting. Which it isn't.

Fix other comment style issues in the file while we are at it.

Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
---
 fs/befs/super.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/befs/super.c b/fs/befs/super.c
index c36745d..80b93c0 100644
--- a/fs/befs/super.c
+++ b/fs/befs/super.c
@@ -13,13 +13,9 @@
 #include "befs.h"
 #include "super.h"
 
-/**
+/*
  * befs_load_sb -- Read from disk and properly byteswap all the fields
  * of the befs superblock
- *
- *
- *
- *
  */
 int
 befs_load_sb(struct super_block *sb, befs_super_block * disk_sb)
@@ -93,8 +89,8 @@ befs_check_sb(struct super_block *sb)
 	}
 
 	/*
-	   * block_shift and block_size encode the same information
-	   * in different ways as a consistency check.
+	 * block_shift and block_size encode the same information
+	 * in different ways as a consistency check.
 	 */
 
 	if ((1 << befs_sb->block_shift) != befs_sb->block_size) {
-- 
2.5.1

Re: [PATCH 3/4] befs: fix comment style

[Salah Triki]

On Tue, Aug 09, 2016 at 11:01:25PM +0100, Luis de Bethencourt wrote:
> The description of befs_load_sb was confusing the kernel-doc system since,
> because it starts with /**, it thinks it will document the function with
> kernel-doc formatting. Which it isn't.
> 
> Fix other comment style issues in the file while we are at it.
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
>  fs/befs/super.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/befs/super.c b/fs/befs/super.c
> index c36745d..80b93c0 100644
> --- a/fs/befs/super.c
> +++ b/fs/befs/super.c
> @@ -13,13 +13,9 @@
>  #include "befs.h"
>  #include "super.h"
>  
> -/**
> +/*
>   * befs_load_sb -- Read from disk and properly byteswap all the fields
>   * of the befs superblock
> - *
> - *
> - *
> - *
>   */
>  int
>  befs_load_sb(struct super_block *sb, befs_super_block * disk_sb)
> @@ -93,8 +89,8 @@ befs_check_sb(struct super_block *sb)
>  	}
>  
>  	/*
> -	   * block_shift and block_size encode the same information
> -	   * in different ways as a consistency check.
> +	 * block_shift and block_size encode the same information
> +	 * in different ways as a consistency check.
>  	 */
>  
>  	if ((1 << befs_sb->block_shift) != befs_sb->block_size) {
> -- 
> 2.5.1
> 

Signed-off-by: Salah Triki <salah.triki@gmail.com>

Thanx
Salah

[PATCH 4/4] befs: fix style issues in super.c

[Luis de Bethencourt]

Fixing the following checkpatch.pl error:

ERROR: "foo * bar" should be "foo *bar"
+befs_load_sb(struct super_block *sb, befs_super_block * disk_sb)

And the following warnings:

WARNING: suspect code indent for conditional statements (8, 12)
+       if (disk_sb->fs_byte_order == BEFS_BYTEORDER_NATIVE_LE)
+           befs_sb->byte_order = BEFS_BYTESEX_LE;

WARNING: suspect code indent for conditional statements (8, 12)
+       else if (disk_sb->fs_byte_order == BEFS_BYTEORDER_NATIVE_BE)
+           befs_sb->byte_order = BEFS_BYTESEX_BE;

WARNING: break quoted strings at a space character
+               befs_error(sb, "blocksize(%u) cannot be larger"
+                          "than system pagesize(%lu)", befs_sb->block_size,

WARNING: line over 80 characters
+       if (befs_sb->log_start != befs_sb->log_end || befs_sb->flags == BEFS_DIRTY) {

Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
---

Fixing these because I was touching this code area.

Thanks,
Luis

 fs/befs/super.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/fs/befs/super.c b/fs/befs/super.c
index 80b93c0..7c50025 100644
--- a/fs/befs/super.c
+++ b/fs/befs/super.c
@@ -18,15 +18,15 @@
  * of the befs superblock
  */
 int
-befs_load_sb(struct super_block *sb, befs_super_block * disk_sb)
+befs_load_sb(struct super_block *sb, befs_super_block *disk_sb)
 {
 	struct befs_sb_info *befs_sb = BEFS_SB(sb);
 
 	/* Check the byte order of the filesystem */
 	if (disk_sb->fs_byte_order == BEFS_BYTEORDER_NATIVE_LE)
-	    befs_sb->byte_order = BEFS_BYTESEX_LE;
+		befs_sb->byte_order = BEFS_BYTESEX_LE;
 	else if (disk_sb->fs_byte_order == BEFS_BYTEORDER_NATIVE_BE)
-	    befs_sb->byte_order = BEFS_BYTESEX_BE;
+		befs_sb->byte_order = BEFS_BYTESEX_BE;
 
 	befs_sb->magic1 = fs32_to_cpu(sb, disk_sb->magic1);
 	befs_sb->magic2 = fs32_to_cpu(sb, disk_sb->magic2);
@@ -82,7 +82,7 @@ befs_check_sb(struct super_block *sb)
 	}
 
 	if (befs_sb->block_size > PAGE_SIZE) {
-		befs_error(sb, "blocksize(%u) cannot be larger"
+		befs_error(sb, "blocksize(%u) cannot be larger "
 			   "than system pagesize(%lu)", befs_sb->block_size,
 			   PAGE_SIZE);
 		return BEFS_ERR;
@@ -106,10 +106,11 @@ befs_check_sb(struct super_block *sb)
 	if ((1 << befs_sb->ag_shift) != befs_sb->blocks_per_ag)
 		befs_error(sb, "ag_shift disagrees with blocks_per_ag.");
 
-	if (befs_sb->log_start != befs_sb->log_end || befs_sb->flags == BEFS_DIRTY) {
+	if (befs_sb->log_start != befs_sb->log_end ||
+	    befs_sb->flags == BEFS_DIRTY) {
 		befs_error(sb, "Filesystem not clean! There are blocks in the "
-			   "journal. You must boot into BeOS and mount this volume "
-			   "to make it clean.");
+			   "journal. You must boot into BeOS and mount this "
+			   "volume to make it clean.");
 		return BEFS_ERR;
 	}
 
-- 
2.5.1

Re: [PATCH 4/4] befs: fix style issues in super.c

[Salah Triki]

On Tue, Aug 09, 2016 at 11:01:26PM +0100, Luis de Bethencourt wrote:
> Fixing the following checkpatch.pl error:
> 
> ERROR: "foo * bar" should be "foo *bar"
> +befs_load_sb(struct super_block *sb, befs_super_block * disk_sb)
> 
> And the following warnings:
> 
> WARNING: suspect code indent for conditional statements (8, 12)
> +       if (disk_sb->fs_byte_order == BEFS_BYTEORDER_NATIVE_LE)
> +           befs_sb->byte_order = BEFS_BYTESEX_LE;
> 
> WARNING: suspect code indent for conditional statements (8, 12)
> +       else if (disk_sb->fs_byte_order == BEFS_BYTEORDER_NATIVE_BE)
> +           befs_sb->byte_order = BEFS_BYTESEX_BE;
> 
> WARNING: break quoted strings at a space character
> +               befs_error(sb, "blocksize(%u) cannot be larger"
> +                          "than system pagesize(%lu)", befs_sb->block_size,
> 
> WARNING: line over 80 characters
> +       if (befs_sb->log_start != befs_sb->log_end || befs_sb->flags == BEFS_DIRTY) {
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
> 
> Fixing these because I was touching this code area.
> 
> Thanks,
> Luis
> 
>  fs/befs/super.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/befs/super.c b/fs/befs/super.c
> index 80b93c0..7c50025 100644
> --- a/fs/befs/super.c
> +++ b/fs/befs/super.c
> @@ -18,15 +18,15 @@
>   * of the befs superblock
>   */
>  int
> -befs_load_sb(struct super_block *sb, befs_super_block * disk_sb)
> +befs_load_sb(struct super_block *sb, befs_super_block *disk_sb)
>  {
>  	struct befs_sb_info *befs_sb = BEFS_SB(sb);
>  
>  	/* Check the byte order of the filesystem */
>  	if (disk_sb->fs_byte_order == BEFS_BYTEORDER_NATIVE_LE)
> -	    befs_sb->byte_order = BEFS_BYTESEX_LE;
> +		befs_sb->byte_order = BEFS_BYTESEX_LE;
>  	else if (disk_sb->fs_byte_order == BEFS_BYTEORDER_NATIVE_BE)
> -	    befs_sb->byte_order = BEFS_BYTESEX_BE;
> +		befs_sb->byte_order = BEFS_BYTESEX_BE;
>  
>  	befs_sb->magic1 = fs32_to_cpu(sb, disk_sb->magic1);
>  	befs_sb->magic2 = fs32_to_cpu(sb, disk_sb->magic2);
> @@ -82,7 +82,7 @@ befs_check_sb(struct super_block *sb)
>  	}
>  
>  	if (befs_sb->block_size > PAGE_SIZE) {
> -		befs_error(sb, "blocksize(%u) cannot be larger"
> +		befs_error(sb, "blocksize(%u) cannot be larger "
>  			   "than system pagesize(%lu)", befs_sb->block_size,
>  			   PAGE_SIZE);
>  		return BEFS_ERR;
> @@ -106,10 +106,11 @@ befs_check_sb(struct super_block *sb)
>  	if ((1 << befs_sb->ag_shift) != befs_sb->blocks_per_ag)
>  		befs_error(sb, "ag_shift disagrees with blocks_per_ag.");
>  
> -	if (befs_sb->log_start != befs_sb->log_end || befs_sb->flags == BEFS_DIRTY) {
> +	if (befs_sb->log_start != befs_sb->log_end ||
> +	    befs_sb->flags == BEFS_DIRTY) {
>  		befs_error(sb, "Filesystem not clean! There are blocks in the "
> -			   "journal. You must boot into BeOS and mount this volume "
> -			   "to make it clean.");
> +			   "journal. You must boot into BeOS and mount this "
> +			   "volume to make it clean.");
>  		return BEFS_ERR;
>  	}
>  
> -- 
> 2.5.1
> 

Signed-off-by: Salah Triki <salah.triki@gmail.com>

Thanx
Salah

Re: [PATCH 1/4] befs: dump inode_size superblock information

[Salah Triki]

On Tue, Aug 09, 2016 at 11:01:23PM +0100, Luis de Bethencourt wrote:
> befs_dump_super_block() wasn't giving the inode_size information when
> dumping all elements of the superblock. Add this element to have complete
> information of the superblock.
> 
> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> ---
> 
> Hi,
> 
> This is a series of superblock information related changes.
> 
> Thanks,
> Luis
> 
>  fs/befs/debug.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/befs/debug.c b/fs/befs/debug.c
> index 4de7cff..85c1339 100644
> --- a/fs/befs/debug.c
> +++ b/fs/befs/debug.c
> @@ -169,6 +169,7 @@ befs_dump_super_block(const struct super_block *sb, befs_super_block * sup)
>  
>  	befs_debug(sb, "  num_blocks %llu", fs64_to_cpu(sb, sup->num_blocks));
>  	befs_debug(sb, "  used_blocks %llu", fs64_to_cpu(sb, sup->used_blocks));
> +	befs_debug(sb, "  inode_size %u", fs32_to_cpu(sb, sup->inode_size));
>  
>  	befs_debug(sb, "  magic2 %08x", fs32_to_cpu(sb, sup->magic2));
>  	befs_debug(sb, "  blocks_per_ag %u",
> -- 
> 2.5.1
> 

Signed-off-by: Salah Triki <salah.triki@gmail.com>

Thanx
Salah

Re: [PATCH 1/4] befs: dump inode_size superblock information

[Luis de Bethencourt]

On 10/08/16 22:41, Salah Triki wrote:
> On Tue, Aug 09, 2016 at 11:01:23PM +0100, Luis de Bethencourt wrote:
>> befs_dump_super_block() wasn't giving the inode_size information when
>> dumping all elements of the superblock. Add this element to have complete
>> information of the superblock.
>>
>> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
>> ---
>>
>> Hi,
>>
>> This is a series of superblock information related changes.
>>
>> Thanks,
>> Luis
>>
>>  fs/befs/debug.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/befs/debug.c b/fs/befs/debug.c
>> index 4de7cff..85c1339 100644
>> --- a/fs/befs/debug.c
>> +++ b/fs/befs/debug.c
>> @@ -169,6 +169,7 @@ befs_dump_super_block(const struct super_block *sb, befs_super_block * sup)
>>  
>>  	befs_debug(sb, "  num_blocks %llu", fs64_to_cpu(sb, sup->num_blocks));
>>  	befs_debug(sb, "  used_blocks %llu", fs64_to_cpu(sb, sup->used_blocks));
>> +	befs_debug(sb, "  inode_size %u", fs32_to_cpu(sb, sup->inode_size));
>>  
>>  	befs_debug(sb, "  magic2 %08x", fs32_to_cpu(sb, sup->magic2));
>>  	befs_debug(sb, "  blocks_per_ag %u",
>> -- 
>> 2.5.1
>>
> 
> Signed-off-by: Salah Triki <salah.triki@gmail.com>
> 
> Thanx
> Salah
> 

Hi Salah,

I prefer applying this series as a set. So will wait for your review of patch 2/4.

Let me know if you don't have time for it.

Thanks for the reviews,
Luis

Re: [PATCH 1/4] befs: dump inode_size superblock information

[Salah Triki]

On Thu, Aug 11, 2016 at 11:03:47AM +0100, Luis de Bethencourt wrote:
> On 10/08/16 22:41, Salah Triki wrote:
> > On Tue, Aug 09, 2016 at 11:01:23PM +0100, Luis de Bethencourt wrote:
> >> befs_dump_super_block() wasn't giving the inode_size information when
> >> dumping all elements of the superblock. Add this element to have complete
> >> information of the superblock.
> >>
> >> Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
> >> ---
> >>
> >> Hi,
> >>
> >> This is a series of superblock information related changes.
> >>
> >> Thanks,
> >> Luis
> >>
> >>  fs/befs/debug.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/fs/befs/debug.c b/fs/befs/debug.c
> >> index 4de7cff..85c1339 100644
> >> --- a/fs/befs/debug.c
> >> +++ b/fs/befs/debug.c
> >> @@ -169,6 +169,7 @@ befs_dump_super_block(const struct super_block *sb, befs_super_block * sup)
> >>  
> >>  	befs_debug(sb, "  num_blocks %llu", fs64_to_cpu(sb, sup->num_blocks));
> >>  	befs_debug(sb, "  used_blocks %llu", fs64_to_cpu(sb, sup->used_blocks));
> >> +	befs_debug(sb, "  inode_size %u", fs32_to_cpu(sb, sup->inode_size));
> >>  
> >>  	befs_debug(sb, "  magic2 %08x", fs32_to_cpu(sb, sup->magic2));
> >>  	befs_debug(sb, "  blocks_per_ag %u",
> >> -- 
> >> 2.5.1
> >>
> > 
> > Signed-off-by: Salah Triki <salah.triki@gmail.com>
> > 
> > Thanx
> > Salah
> > 
> 
> Hi Salah,
> 
> I prefer applying this series as a set. So will wait for your review of patch 2/4.
> 
> Let me know if you don't have time for it.
> 
> Thanks for the reviews,
> Luis

Hi Luis,

I am reviewing the patch 2/4 

regards,
Salah