Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1715578 > unrolled thread

[PATCH 0/4] ISOFS: Adjustments for three function implementations

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] ISOFS: Adjustments for three function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-19 09:20 +0200
    [PATCH 3/4] isofs: One check less in isofs_read_inode() after error  detection SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-19 09:20 +0200
      Re: [PATCH 3/4] isofs: One check less in isofs_read_inode() after  error detection Jan Kara <jack@suse.cz> - 2017-08-21 16:20 +0200
        Re: isofs: One check less in isofs_read_inode() after error detection SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-21 16:20 +0200
          Re: isofs: One check less in isofs_read_inode() after error detection Jan Kara <jack@suse.cz> - 2017-08-22 12:50 +0200
    [PATCH 4/4] isofs: Delete an unnecessary variable initialisation in  isofs_read_inode() SF Markus Elfring <elfring@users.sourceforge.net> - 2017-08-19 09:20 +0200
      Re: [PATCH 4/4] isofs: Delete an unnecessary variable initialisation  in isofs_read_inode() Jan Kara <jack@suse.cz> - 2017-08-21 16:20 +0200

#1715578 — [PATCH 0/4] ISOFS: Adjustments for three function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 19 Aug 2017 08:40:12 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Delete an error message for a failed memory allocation in isofs_read_inode()
  Adjust four checks for null pointers
  One check less in isofs_read_inode() after error detection
  Delete an unnecessary variable initialisation in isofs_read_inode()

 fs/isofs/inode.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

-- 
2.14.0

[toc] | [next] | [standalone]

#1715579 — [PATCH 3/4] isofs: One check less in isofs_read_inode() after error detection

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 19 Aug 2017 08:10:40 +0200

* Adjust jump targets.

* Avoid a repeated check for the local variable "bh" after
  a memory allocation failure in this function.

* Delete an initialisation for the local variable "ret"
  which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/isofs/inode.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index db43f52f2268..f6d6c2ca8723 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -1304,7 +1304,7 @@ static int isofs_read_inode(struct inode *inode, int relocated)
 	unsigned int de_len;
 	unsigned long offset;
 	struct iso_inode_info *ei = ISOFS_I(inode);
-	int ret = -EIO;
+	int ret;
 
 	block = ei->i_iget5_block;
 	bh = sb_bread(inode->i_sb, block);
@@ -1322,7 +1322,7 @@ static int isofs_read_inode(struct inode *inode, int relocated)
 		tmpde = kmalloc(de_len, GFP_KERNEL);
 		if (!tmpde) {
 			ret = -ENOMEM;
-			goto fail;
+			goto release_bh;
 		}
 		memcpy(tmpde, bh->b_data + offset, frag1);
 		brelse(bh);
@@ -1377,7 +1377,7 @@ static int isofs_read_inode(struct inode *inode, int relocated)
 	if (de->flags[-high_sierra] & 0x80) {
 		ret = isofs_read_level3_size(inode);
 		if (ret < 0)
-			goto fail;
+			goto free_tmp;
 		ret = -EIO;
 	} else {
 		ei->i_next_section_block = 0;
@@ -1475,16 +1475,17 @@ static int isofs_read_inode(struct inode *inode, int relocated)
 		init_special_inode(inode, inode->i_mode, inode->i_rdev);
 
 	ret = 0;
-out:
+free_tmp:
 	kfree(tmpde);
 	if (bh)
+release_bh:
 		brelse(bh);
 	return ret;
 
 out_badread:
 	printk(KERN_WARNING "ISOFS: unable to read i-node block\n");
-fail:
-	goto out;
+	ret = -EIO;
+	goto free_tmp;
 }
 
 struct isofs_iget5_callback_data {
-- 
2.14.0

[toc] | [prev] | [next] | [standalone]

#1716531 — Re: [PATCH 3/4] isofs: One check less in isofs_read_inode() after error detection

On Sat 19-08-17 09:18:42, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 19 Aug 2017 08:10:40 +0200
> 
> * Adjust jump targets.
> 
> * Avoid a repeated check for the local variable "bh" after
>   a memory allocation failure in this function.
> 
> * Delete an initialisation for the local variable "ret"
>   which became unnecessary with this refactoring.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

I agree that

fail:
        goto out;

in the original code is awkward and we can get rid of it. However renaming
labels is IMO pointless and

>  	if (bh)
> +release_bh:
>  		brelse(bh);
>  	return ret;

is just disgusting.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1716539 — Re: isofs: One check less in isofs_read_inode() after error detection

> I agree that
> 
> fail:
>         goto out;
> 
> in the original code is awkward and we can get rid of it.

How would you like to change this place instead?


> However renaming labels is IMO pointless and
> 
>>  	if (bh)
>> +release_bh:
>>  		brelse(bh);
>>  	return ret;
> 
> is just disgusting.

I know that it can be occasionally harder to achieve the desired consensus.

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1717300 — Re: isofs: One check less in isofs_read_inode() after error detection

On Mon 21-08-17 16:18:14, SF Markus Elfring wrote:
> > I agree that
> > 
> > fail:
> >         goto out;
> > 
> > in the original code is awkward and we can get rid of it.
> 
> How would you like to change this place instead?

Just do 'goto out' directly.

								Honza

-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1715580 — [PATCH 4/4] isofs: Delete an unnecessary variable initialisation in isofs_read_inode()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 19 Aug 2017 08:13:46 +0200

The local variable "bh" will be set to an appropriate pointer a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 fs/isofs/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index f6d6c2ca8723..3a33b3db712a 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -1298,7 +1298,7 @@ static int isofs_read_inode(struct inode *inode, int relocated)
 	unsigned long bufsize = ISOFS_BUFFER_SIZE(inode);
 	unsigned long block;
 	int high_sierra = sbi->s_high_sierra;
-	struct buffer_head *bh = NULL;
+	struct buffer_head *bh;
 	struct iso_directory_record *de;
 	struct iso_directory_record *tmpde = NULL;
 	unsigned int de_len;
-- 
2.14.0

[toc] | [prev] | [next] | [standalone]

#1716535 — Re: [PATCH 4/4] isofs: Delete an unnecessary variable initialisation in isofs_read_inode()

On Sat 19-08-17 09:19:34, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 19 Aug 2017 08:13:46 +0200
> 
> The local variable "bh" will be set to an appropriate pointer a bit later.
> Thus omit the explicit initialisation at the beginning.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Applied to my tree. Thanks.

								Honza
> ---
>  fs/isofs/inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index f6d6c2ca8723..3a33b3db712a 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -1298,7 +1298,7 @@ static int isofs_read_inode(struct inode *inode, int relocated)
>  	unsigned long bufsize = ISOFS_BUFFER_SIZE(inode);
>  	unsigned long block;
>  	int high_sierra = sbi->s_high_sierra;
> -	struct buffer_head *bh = NULL;
> +	struct buffer_head *bh;
>  	struct iso_directory_record *de;
>  	struct iso_directory_record *tmpde = NULL;
>  	unsigned int de_len;
> -- 
> 2.14.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web