* [PATCH 1/4] isofs: Delete an error message for a failed memory allocation in isofs_read_inode()
2017-08-19 7:14 [PATCH 0/4] ISOFS: Adjustments for three function implementations SF Markus Elfring
@ 2017-08-19 7:15 ` SF Markus Elfring
2017-08-21 14:06 ` Jan Kara
2017-08-19 7:17 ` [PATCH 2/4] isofs: Adjust four checks for null pointers SF Markus Elfring
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-19 7:15 UTC (permalink / raw)
To: Al Viro, David Howells, Ingo Molnar, Jan Kara, kernel-janitors; +Cc: LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 18 Aug 2017 21:41:24 +0200
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
fs/isofs/inode.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index 217a5e7815da..2d3f090944df 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -1324,4 +1324,3 @@ static int isofs_read_inode(struct inode *inode, int relocated)
- printk(KERN_INFO "%s: out of memory\n", __func__);
ret = -ENOMEM;
goto fail;
}
--
2.14.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/4] isofs: Delete an error message for a failed memory allocation in isofs_read_inode()
2017-08-19 7:15 ` [PATCH 1/4] isofs: Delete an error message for a failed memory allocation in isofs_read_inode() SF Markus Elfring
@ 2017-08-21 14:06 ` Jan Kara
0 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2017-08-21 14:06 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Al Viro, David Howells, Ingo Molnar, Jan Kara, kernel-janitors,
LKML
On Sat 19-08-17 09:15:57, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 18 Aug 2017 21:41:24 +0200
>
> Omit an extra message for a memory allocation failure in this function.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Thanks. Applied to my tree.
Honza
> ---
> fs/isofs/inode.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index 217a5e7815da..2d3f090944df 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -1324,4 +1324,3 @@ static int isofs_read_inode(struct inode *inode, int relocated)
> - printk(KERN_INFO "%s: out of memory\n", __func__);
> ret = -ENOMEM;
> goto fail;
> }
> --
> 2.14.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/4] isofs: Adjust four checks for null pointers
2017-08-19 7:14 [PATCH 0/4] ISOFS: Adjustments for three function implementations SF Markus Elfring
2017-08-19 7:15 ` [PATCH 1/4] isofs: Delete an error message for a failed memory allocation in isofs_read_inode() SF Markus Elfring
@ 2017-08-19 7:17 ` SF Markus Elfring
2017-08-21 14:06 ` Jan Kara
2017-08-19 7:18 ` [PATCH 3/4] isofs: One check less in isofs_read_inode() after error detection SF Markus Elfring
2017-08-19 7:19 ` [PATCH 4/4] isofs: Delete an unnecessary variable initialisation in isofs_read_inode() SF Markus Elfring
3 siblings, 1 reply; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-19 7:17 UTC (permalink / raw)
To: Al Viro, David Howells, Ingo Molnar, Jan Kara, kernel-janitors; +Cc: LKML
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 18 Aug 2017 21:47:14 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The script “checkpatch.pl” pointed information out like the following.
Comparison to NULL could be written !…
Thus fix the affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
fs/isofs/inode.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
index 2d3f090944df..db43f52f2268 100644
--- a/fs/isofs/inode.c
+++ b/fs/isofs/inode.c
@@ -96,7 +96,7 @@ static int __init init_inodecache(void)
0, (SLAB_RECLAIM_ACCOUNT|
SLAB_MEM_SPREAD|SLAB_ACCOUNT),
init_once);
- if (isofs_inode_cachep = NULL)
+ if (!isofs_inode_cachep)
return -ENOMEM;
return 0;
}
@@ -678,7 +678,7 @@ static int isofs_fill_super(struct super_block *s, void *data, int silent)
if (isonum_711(vdp->type) = ISO_VD_END)
break;
if (isonum_711(vdp->type) = ISO_VD_PRIMARY) {
- if (pri = NULL) {
+ if (!pri) {
pri = (struct iso_primary_descriptor *)vdp;
/* Save the buffer in case we need it ... */
pri_bh = bh;
@@ -742,7 +742,7 @@ static int isofs_fill_super(struct super_block *s, void *data, int silent)
goto out_freebh;
}
- if (joliet_level && (pri = NULL || !opt.rock)) {
+ if (joliet_level && (!pri || !opt.rock)) {
/* This is the case of Joliet with the norock mount flag.
* A disc with both Joliet and Rock Ridge is handled later
*/
@@ -1320,5 +1320,5 @@ static int isofs_read_inode(struct inode *inode, int relocated)
int frag1 = bufsize - offset;
tmpde = kmalloc(de_len, GFP_KERNEL);
- if (tmpde = NULL) {
+ if (!tmpde) {
ret = -ENOMEM;
--
2.14.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/4] isofs: Adjust four checks for null pointers
2017-08-19 7:17 ` [PATCH 2/4] isofs: Adjust four checks for null pointers SF Markus Elfring
@ 2017-08-21 14:06 ` Jan Kara
0 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2017-08-21 14:06 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Al Viro, David Howells, Ingo Molnar, Jan Kara, kernel-janitors,
LKML
On Sat 19-08-17 09:17:28, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Fri, 18 Aug 2017 21:47:14 +0200
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> The script “checkpatch.pl” pointed information out like the following.
>
> Comparison to NULL could be written !…
>
> Thus fix the affected source code places.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Thanks. Applied to my tree.
Honza
> ---
> fs/isofs/inode.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/isofs/inode.c b/fs/isofs/inode.c
> index 2d3f090944df..db43f52f2268 100644
> --- a/fs/isofs/inode.c
> +++ b/fs/isofs/inode.c
> @@ -96,7 +96,7 @@ static int __init init_inodecache(void)
> 0, (SLAB_RECLAIM_ACCOUNT|
> SLAB_MEM_SPREAD|SLAB_ACCOUNT),
> init_once);
> - if (isofs_inode_cachep = NULL)
> + if (!isofs_inode_cachep)
> return -ENOMEM;
> return 0;
> }
> @@ -678,7 +678,7 @@ static int isofs_fill_super(struct super_block *s, void *data, int silent)
> if (isonum_711(vdp->type) = ISO_VD_END)
> break;
> if (isonum_711(vdp->type) = ISO_VD_PRIMARY) {
> - if (pri = NULL) {
> + if (!pri) {
> pri = (struct iso_primary_descriptor *)vdp;
> /* Save the buffer in case we need it ... */
> pri_bh = bh;
> @@ -742,7 +742,7 @@ static int isofs_fill_super(struct super_block *s, void *data, int silent)
> goto out_freebh;
> }
>
> - if (joliet_level && (pri = NULL || !opt.rock)) {
> + if (joliet_level && (!pri || !opt.rock)) {
> /* This is the case of Joliet with the norock mount flag.
> * A disc with both Joliet and Rock Ridge is handled later
> */
> @@ -1320,5 +1320,5 @@ static int isofs_read_inode(struct inode *inode, int relocated)
> int frag1 = bufsize - offset;
>
> tmpde = kmalloc(de_len, GFP_KERNEL);
> - if (tmpde = NULL) {
> + if (!tmpde) {
> ret = -ENOMEM;
> --
> 2.14.0
>
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/4] isofs: One check less in isofs_read_inode() after error detection
2017-08-19 7:14 [PATCH 0/4] ISOFS: Adjustments for three function implementations SF Markus Elfring
2017-08-19 7:15 ` [PATCH 1/4] isofs: Delete an error message for a failed memory allocation in isofs_read_inode() SF Markus Elfring
2017-08-19 7:17 ` [PATCH 2/4] isofs: Adjust four checks for null pointers SF Markus Elfring
@ 2017-08-19 7:18 ` SF Markus Elfring
2017-08-21 14:11 ` Jan Kara
2017-08-19 7:19 ` [PATCH 4/4] isofs: Delete an unnecessary variable initialisation in isofs_read_inode() SF Markus Elfring
3 siblings, 1 reply; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-19 7:18 UTC (permalink / raw)
To: Al Viro, David Howells, Ingo Molnar, Jan Kara, kernel-janitors; +Cc: LKML
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/4] isofs: One check less in isofs_read_inode() after error detection
2017-08-19 7:18 ` [PATCH 3/4] isofs: One check less in isofs_read_inode() after error detection SF Markus Elfring
@ 2017-08-21 14:11 ` Jan Kara
2017-08-21 14:18 ` SF Markus Elfring
0 siblings, 1 reply; 11+ messages in thread
From: Jan Kara @ 2017-08-21 14:11 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Al Viro, David Howells, Ingo Molnar, Jan Kara, kernel-janitors,
LKML
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: isofs: One check less in isofs_read_inode() after error detection
2017-08-21 14:11 ` Jan Kara
@ 2017-08-21 14:18 ` SF Markus Elfring
2017-08-22 10:48 ` Jan Kara
0 siblings, 1 reply; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-21 14:18 UTC (permalink / raw)
To: Jan Kara; +Cc: Al Viro, David Howells, Ingo Molnar, kernel-janitors, LKML
> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: isofs: One check less in isofs_read_inode() after error detection
2017-08-21 14:18 ` SF Markus Elfring
@ 2017-08-22 10:48 ` Jan Kara
0 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2017-08-22 10:48 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Jan Kara, Al Viro, David Howells, Ingo Molnar, kernel-janitors,
LKML
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/4] isofs: Delete an unnecessary variable initialisation in isofs_read_inode()
2017-08-19 7:14 [PATCH 0/4] ISOFS: Adjustments for three function implementations SF Markus Elfring
` (2 preceding siblings ...)
2017-08-19 7:18 ` [PATCH 3/4] isofs: One check less in isofs_read_inode() after error detection SF Markus Elfring
@ 2017-08-19 7:19 ` SF Markus Elfring
2017-08-21 14:12 ` Jan Kara
3 siblings, 1 reply; 11+ messages in thread
From: SF Markus Elfring @ 2017-08-19 7:19 UTC (permalink / raw)
To: Al Viro, David Howells, Ingo Molnar, Jan Kara, kernel-janitors; +Cc: LKML
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 4/4] isofs: Delete an unnecessary variable initialisation in isofs_read_inode()
2017-08-19 7:19 ` [PATCH 4/4] isofs: Delete an unnecessary variable initialisation in isofs_read_inode() SF Markus Elfring
@ 2017-08-21 14:12 ` Jan Kara
0 siblings, 0 replies; 11+ messages in thread
From: Jan Kara @ 2017-08-21 14:12 UTC (permalink / raw)
To: SF Markus Elfring
Cc: Al Viro, David Howells, Ingo Molnar, Jan Kara, kernel-janitors,
LKML
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
^ permalink raw reply [flat|nested] 11+ messages in thread