[PATCH 0/8] Various cleanups

[PATCH 0/8] Various cleanups

[Damien Le Moal]

Mike,

These patches fix various compilation warnings showing up when compiling
with W=1. The last patch addresses a static checker warning (C=1
compilation). There are a lot more of these code checker warnings
remaining that probably could be addressed, some seem to be false
positive though.

Damien Le Moal (8):
  dm raid5: Remove set but unused variable
  dm raid5: Fix compilation warning
  md: Fix compilation warning
  dm raid10: Fix compilation warning
  dm verity: Fix compilation warning
  dm raid: Remove empty if statement
  dm ioctl: Fix compilation warning
  dm init: Set file local variable static

 drivers/md/dm-init.c              |  2 +-
 drivers/md/dm-ioctl.c             |  2 +-
 drivers/md/dm-raid.c              |  2 --
 drivers/md/dm-verity-verify-sig.h | 14 +++++++-------
 drivers/md/md.c                   | 12 ++++++------
 drivers/md/raid10.c               |  4 ++--
 drivers/md/raid5-cache.c          |  4 +---
 drivers/md/raid5.c                | 12 ++++++------
 8 files changed, 24 insertions(+), 28 deletions(-)

-- 
2.26.2

[PATCH 1/8] dm raid5: Remove set but unused variable

[Damien Le Moal]

Remove the variable offset in r5c_tree_index() to avoid a "set but not
used" compilation warning when compiling with W=1.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/md/raid5-cache.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/md/raid5-cache.c b/drivers/md/raid5-cache.c
index 9b6da759dca2..eea14fa9e59b 100644
--- a/drivers/md/raid5-cache.c
+++ b/drivers/md/raid5-cache.c
@@ -195,9 +195,7 @@ struct r5l_log {
 static inline sector_t r5c_tree_index(struct r5conf *conf,
 				      sector_t sect)
 {
-	sector_t offset;
-
-	offset = sector_div(sect, conf->chunk_sectors);
+	sector_div(sect, conf->chunk_sectors);
 	return sect;
 }
 
-- 
2.26.2

[PATCH 2/8] dm raid5: Fix compilation warning

[Damien Le Moal]

Remove the if statement around the calls to sysfs_link_rdev() to avoid
the compilation warning "suggest braces around empty body in an ‘if’
statement" when compiling with W=1.

Also fix function description comments to avoid kdoc format warnings.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/md/raid5.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ab8067f9ce8c..5cd1dd44881c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -2217,9 +2217,9 @@ static int grow_stripes(struct r5conf *conf, int num)
 /**
  * scribble_alloc - allocate percpu scribble buffer for required size
  *		    of the scribble region
- * @percpu - from for_each_present_cpu() of the caller
- * @num - total number of disks in the array
- * @cnt - scribble objs count for required size of the scribble region
+ * @percpu: from for_each_present_cpu() of the caller
+ * @num: total number of disks in the array
+ * @cnt: scribble objs count for required size of the scribble region
  *
  * The scribble buffer size must be enough to contain:
  * 1/ a struct page pointer for each device in the array +2
@@ -3710,7 +3710,7 @@ static int fetch_block(struct stripe_head *sh, struct stripe_head_state *s,
 	return 0;
 }
 
-/**
+/*
  * handle_stripe_fill - read or compute data to satisfy pending requests.
  */
 static void handle_stripe_fill(struct stripe_head *sh,
@@ -7944,8 +7944,8 @@ static int raid5_start_reshape(struct mddev *mddev)
 					else
 						rdev->recovery_offset = 0;
 
-					if (sysfs_link_rdev(mddev, rdev))
-						/* Failure here is OK */;
+					/* Failure here is OK */
+					sysfs_link_rdev(mddev, rdev);
 				}
 			} else if (rdev->raid_disk >= conf->previous_raid_disks
 				   && !test_bit(Faulty, &rdev->flags)) {
-- 
2.26.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH 3/8] md: Fix compilation warning

[Damien Le Moal]

Remove the if statement around the calls to sysfs_link_rdev() to avoid
the compilation warnings:

warning: suggest braces around empty body in an ‘if’ statement

when compiling with W=1. For the call to sysfs_create_link() generating
the same warning, use the err variable to store the function result,
avoiding triggering another warning as the function is declared
as 'warn_unused_result'.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/md/md.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index f567f536b529..f6c83483a5a2 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -2441,8 +2441,8 @@ static int bind_rdev_to_array(struct md_rdev *rdev, struct mddev *mddev)
 		goto fail;
 
 	ko = &part_to_dev(rdev->bdev->bd_part)->kobj;
-	if (sysfs_create_link(&rdev->kobj, ko, "block"))
-		/* failure here is OK */;
+	/* failure here is OK */
+	err = sysfs_create_link(&rdev->kobj, ko, "block");
 	rdev->sysfs_state = sysfs_get_dirent_safe(rdev->kobj.sd, "state");
 
 	list_add_rcu(&rdev->same_set, &mddev->disks);
@@ -3202,8 +3202,8 @@ slot_store(struct md_rdev *rdev, const char *buf, size_t len)
 			return err;
 		} else
 			sysfs_notify_dirent_safe(rdev->sysfs_state);
-		if (sysfs_link_rdev(rdev->mddev, rdev))
-			/* failure here is OK */;
+		/* failure here is OK */;
+		sysfs_link_rdev(rdev->mddev, rdev);
 		/* don't wakeup anyone, leave that to userspace. */
 	} else {
 		if (slot >= rdev->mddev->raid_disks &&
@@ -9060,8 +9060,8 @@ static int remove_and_add_spares(struct mddev *mddev,
 			rdev->recovery_offset = 0;
 		}
 		if (mddev->pers->hot_add_disk(mddev, rdev) == 0) {
-			if (sysfs_link_rdev(mddev, rdev))
-				/* failure here is OK */;
+			/* failure here is OK */
+			sysfs_link_rdev(mddev, rdev);
 			if (!test_bit(Journal, &rdev->flags))
 				spares++;
 			md_new_event(mddev);
-- 
2.26.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH 4/8] dm raid10: Fix compilation warning

[Damien Le Moal]

Remove the if statement around the call to sysfs_link_rdev() in
raid10_start_reshape() to avoid the compilation warning:

warning: suggest braces around empty body in an ‘if’ statement

when compiling with W=1.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/md/raid10.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index ec136e44aef7..08f697f3ddc8 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -4307,8 +4307,8 @@ static int raid10_start_reshape(struct mddev *mddev)
 					else
 						rdev->recovery_offset = 0;
 
-					if (sysfs_link_rdev(mddev, rdev))
-						/* Failure here  is OK */;
+					/* Failure here is OK */
+					sysfs_link_rdev(mddev, rdev);
 				}
 			} else if (rdev->raid_disk >= conf->prev.raid_disks
 				   && !test_bit(Faulty, &rdev->flags)) {
-- 
2.26.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH 5/8] dm verity: Fix compilation warning

[Damien Le Moal]

For the case !CONFIG_DM_VERITY_VERIFY_ROOTHASH_SIG, declare the
functions verity_verify_root_hash(), verity_verify_is_sig_opt_arg(),
verity_verify_sig_parse_opt_args() and verity_verify_sig_opts_cleanup()
as inline to avoid a "no previous prototype for xxx" compilation
warning when compiling with W=1.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/md/dm-verity-verify-sig.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm-verity-verify-sig.h b/drivers/md/dm-verity-verify-sig.h
index 19b1547aa741..3987c7141f79 100644
--- a/drivers/md/dm-verity-verify-sig.h
+++ b/drivers/md/dm-verity-verify-sig.h
@@ -34,25 +34,25 @@ void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts);
 
 #define DM_VERITY_ROOT_HASH_VERIFICATION_OPTS 0
 
-int verity_verify_root_hash(const void *data, size_t data_len,
-			    const void *sig_data, size_t sig_len)
+static inline int verity_verify_root_hash(const void *data, size_t data_len,
+					  const void *sig_data, size_t sig_len)
 {
 	return 0;
 }
 
-bool verity_verify_is_sig_opt_arg(const char *arg_name)
+static inline bool verity_verify_is_sig_opt_arg(const char *arg_name)
 {
 	return false;
 }
 
-int verity_verify_sig_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
-				    struct dm_verity_sig_opts *sig_opts,
-				    unsigned int *argc, const char *arg_name)
+static inline int verity_verify_sig_parse_opt_args(struct dm_arg_set *as,
+			struct dm_verity *v, struct dm_verity_sig_opts *sig_opts,
+			unsigned int *argc, const char *arg_name)
 {
 	return -EINVAL;
 }
 
-void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts)
+static inline void verity_verify_sig_opts_cleanup(struct dm_verity_sig_opts *sig_opts)
 {
 }
 
-- 
2.26.2

[PATCH 6/8] dm raid: Remove empty if statement

[Damien Le Moal]

In super_init_validation(), remove a body-less if statement testing only
variables to avoid a compilation warning when compiling with W=1.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/md/dm-raid.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index 10e8b2fe787b..08023c50aaa0 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -2345,8 +2345,6 @@ static int super_init_validation(struct raid_set *rs, struct md_rdev *rdev)
 
 	if (new_devs == rs->raid_disks || !rebuilds) {
 		/* Replace a broken device */
-		if (new_devs == 1 && !rs->delta_disks)
-			;
 		if (new_devs == rs->raid_disks) {
 			DMINFO("Superblocks created for new raid set");
 			set_bit(MD_ARRAY_FIRST_USE, &mddev->flags);
-- 
2.26.2

[PATCH 7/8] dm ioctl: Fix compilation warning

[Damien Le Moal]

In retrieve_status(), when copying the target type name in the
target_type string field of struct dm_target_spec, copy at most
DM_MAX_TYPE_NAME - 1 character to avoid the compilation warning:

warning: ‘__builtin_strncpy’ specified bound 16 equals destination
size [-Wstringop-truncation]

when compiling with W-1.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/md/dm-ioctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-ioctl.c b/drivers/md/dm-ioctl.c
index 489935d5f22d..5792878dbff6 100644
--- a/drivers/md/dm-ioctl.c
+++ b/drivers/md/dm-ioctl.c
@@ -1168,7 +1168,7 @@ static void retrieve_status(struct dm_table *table,
 		spec->sector_start = ti->begin;
 		spec->length = ti->len;
 		strncpy(spec->target_type, ti->type->name,
-			sizeof(spec->target_type));
+			sizeof(spec->target_type) - 1);
 
 		outptr += sizeof(struct dm_target_spec);
 		remaining = len - (outptr - outbuf);
-- 
2.26.2

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH 8/8] dm init: Set file local variable static

[Damien Le Moal]

Declare dm_allowed_targets as static to avoid the warning:

drivers/md/dm-init.c:39:12: warning: symbol 'dm_allowed_targets' was
not declared. Should it be static?

when compiling with C=1.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 drivers/md/dm-init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/dm-init.c b/drivers/md/dm-init.c
index b869316d3722..b0c45c6ebe0b 100644
--- a/drivers/md/dm-init.c
+++ b/drivers/md/dm-init.c
@@ -36,7 +36,7 @@ struct dm_device {
 	struct list_head list;
 };
 
-const char * const dm_allowed_targets[] __initconst = {
+static const char * const dm_allowed_targets[] __initconst = {
 	"crypt",
 	"delay",
 	"linear",
-- 
2.26.2

Re: [PATCH 0/8] Various cleanups

[Mike Snitzer]

On Wed, Jul 15 2020 at  5:52am -0400,
Damien Le Moal <damien.lemoal@wdc.com> wrote:

> Mike,
> 
> These patches fix various compilation warnings showing up when compiling
> with W=1. The last patch addresses a static checker warning (C=1
> compilation). There are a lot more of these code checker warnings
> remaining that probably could be addressed, some seem to be false
> positive though.
> 
> Damien Le Moal (8):
>   dm raid5: Remove set but unused variable
>   dm raid5: Fix compilation warning
>   md: Fix compilation warning
>   dm raid10: Fix compilation warning
>   dm verity: Fix compilation warning
>   dm raid: Remove empty if statement
>   dm ioctl: Fix compilation warning
>   dm init: Set file local variable static
> 
>  drivers/md/dm-init.c              |  2 +-
>  drivers/md/dm-ioctl.c             |  2 +-
>  drivers/md/dm-raid.c              |  2 --
>  drivers/md/dm-verity-verify-sig.h | 14 +++++++-------

OK, thanks.

>  drivers/md/md.c                   | 12 ++++++------
>  drivers/md/raid10.c               |  4 ++--
>  drivers/md/raid5-cache.c          |  4 +---
>  drivers/md/raid5.c                | 12 ++++++------

But these need to be routed through the MD.

Mike

Re: [PATCH 0/8] Various cleanups

[Damien Le Moal]

On 2020/07/15 22:43, Mike Snitzer wrote:
> On Wed, Jul 15 2020 at  5:52am -0400,
> Damien Le Moal <damien.lemoal@wdc.com> wrote:
> 
>> Mike,
>>
>> These patches fix various compilation warnings showing up when compiling
>> with W=1. The last patch addresses a static checker warning (C=1
>> compilation). There are a lot more of these code checker warnings
>> remaining that probably could be addressed, some seem to be false
>> positive though.
>>
>> Damien Le Moal (8):
>>   dm raid5: Remove set but unused variable
>>   dm raid5: Fix compilation warning
>>   md: Fix compilation warning
>>   dm raid10: Fix compilation warning
>>   dm verity: Fix compilation warning
>>   dm raid: Remove empty if statement
>>   dm ioctl: Fix compilation warning
>>   dm init: Set file local variable static
>>
>>  drivers/md/dm-init.c              |  2 +-
>>  drivers/md/dm-ioctl.c             |  2 +-
>>  drivers/md/dm-raid.c              |  2 --
>>  drivers/md/dm-verity-verify-sig.h | 14 +++++++-------
> 
> OK, thanks.
> 
>>  drivers/md/md.c                   | 12 ++++++------
>>  drivers/md/raid10.c               |  4 ++--
>>  drivers/md/raid5-cache.c          |  4 +---
>>  drivers/md/raid5.c                | 12 ++++++------
> 
> But these need to be routed through the MD.

Oops. Yes ! Will do. Thanks !

> 
> Mike
> 
> 


-- 
Damien Le Moal
Western Digital Research