[PATCH] staging: kpc2000: Use memset to initialize resources

[PATCH] staging: kpc2000: Use memset to initialize resources

[Nathan Chancellor]

Clang warns:

drivers/staging/kpc2000/kpc2000/cell_probe.c:96:38: warning: suggest
braces around initialization of subobject [-Wmissing-braces]
    struct resource  resources[2] = {0};
                                     ^
                                     {}
drivers/staging/kpc2000/kpc2000/cell_probe.c:314:38: warning: suggest
braces around initialization of subobject [-Wmissing-braces]
    struct resource  resources[2] = {0};
                                     ^
                                     {}
2 warnings generated.

One way to fix these warnings is to add additional braces like Clang
suggests; however, there has been a bit of push back from some
maintainers, who just prefer memset as it is unambiguous, doesn't
depend on a particular compiler version, and properly initializes all
subobjects [1][2]. Do that here so there are no more warnings.

[1]: https://lore.kernel.org/lkml/022e41c0-8465-dc7a-a45c-64187ecd9684@amd.com/
[2]: https://lore.kernel.org/lkml/20181128.215241.702406654469517539.davem@davemloft.net/

Link: https://github.com/ClangBuiltLinux/linux/issues/455
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 drivers/staging/kpc2000/kpc2000/cell_probe.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c b/drivers/staging/kpc2000/kpc2000/cell_probe.c
index ad2cc0a3bfa1..13f544f3c0b9 100644
--- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
+++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
@@ -93,8 +93,8 @@ void parse_core_table_entry(struct core_table_entry *cte, const u64 read_val, co
 int  probe_core_basic(unsigned int core_num, struct kp2000_device *pcard, char *name, const struct core_table_entry cte)
 {
     struct mfd_cell  cell = {0};
-    struct resource  resources[2] = {0};
-    
+    struct resource  resources[2];
+
     struct kpc_core_device_platdata  core_pdata = {
         .card_id           = pcard->card_id,
         .build_version     = pcard->build_version,
@@ -112,6 +112,8 @@ int  probe_core_basic(unsigned int core_num, struct kp2000_device *pcard, char *
     cell.id = core_num;
     cell.num_resources = 2;
     
+    memset(&resources, 0, sizeof(resources));
+
     resources[0].start = cte.offset;
     resources[0].end   = cte.offset + (cte.length - 1);
     resources[0].flags = IORESOURCE_MEM;
@@ -311,8 +313,8 @@ int  probe_core_uio(unsigned int core_num, struct kp2000_device *pcard, char *na
 static int  create_dma_engine_core(struct kp2000_device *pcard, size_t engine_regs_offset, int engine_num, int irq_num)
 {
     struct mfd_cell  cell = {0};
-    struct resource  resources[2] = {0};
-    
+    struct resource  resources[2];
+
     dev_dbg(&pcard->pdev->dev, "create_dma_core(pcard = [%p], engine_regs_offset = %zx, engine_num = %d)\n", pcard, engine_regs_offset, engine_num);
     
     cell.platform_data = NULL;
@@ -321,6 +323,8 @@ static int  create_dma_engine_core(struct kp2000_device *pcard, size_t engine_re
     cell.name = KP_DRIVER_NAME_DMA_CONTROLLER;
     cell.num_resources = 2;
     
+    memset(&resources, 0, sizeof(resources));
+
     resources[0].start = engine_regs_offset;
     resources[0].end   = engine_regs_offset + (KPC_DMA_ENGINE_SIZE - 1);
     resources[0].flags = IORESOURCE_MEM;
-- 
2.21.0

Re: [PATCH] staging: kpc2000: Use memset to initialize resources

[Nick Desaulniers]

On Wed, Apr 24, 2019 at 11:58 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Clang warns:
>
> drivers/staging/kpc2000/kpc2000/cell_probe.c:96:38: warning: suggest
> braces around initialization of subobject [-Wmissing-braces]
>     struct resource  resources[2] = {0};
>                                      ^
>                                      {}
> drivers/staging/kpc2000/kpc2000/cell_probe.c:314:38: warning: suggest
> braces around initialization of subobject [-Wmissing-braces]
>     struct resource  resources[2] = {0};
>                                      ^
>                                      {}
> 2 warnings generated.
>
> One way to fix these warnings is to add additional braces like Clang
> suggests; however, there has been a bit of push back from some
> maintainers, who just prefer memset as it is unambiguous, doesn't
> depend on a particular compiler version, and properly initializes all
> subobjects [1][2]. Do that here so there are no more warnings.
>
> [1]: https://lore.kernel.org/lkml/022e41c0-8465-dc7a-a45c-64187ecd9684@amd.com/
> [2]: https://lore.kernel.org/lkml/20181128.215241.702406654469517539.davem@davemloft.net/
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/455
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
>  drivers/staging/kpc2000/kpc2000/cell_probe.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> index ad2cc0a3bfa1..13f544f3c0b9 100644
> --- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
> +++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> @@ -93,8 +93,8 @@ void parse_core_table_entry(struct core_table_entry *cte, const u64 read_val, co
>  int  probe_core_basic(unsigned int core_num, struct kp2000_device *pcard, char *name, const struct core_table_entry cte)
>  {
>      struct mfd_cell  cell = {0};
> -    struct resource  resources[2] = {0};
> -
> +    struct resource  resources[2];
> +
>      struct kpc_core_device_platdata  core_pdata = {
>          .card_id           = pcard->card_id,
>          .build_version     = pcard->build_version,
> @@ -112,6 +112,8 @@ int  probe_core_basic(unsigned int core_num, struct kp2000_device *pcard, char *
>      cell.id = core_num;
>      cell.num_resources = 2;
>
> +    memset(&resources, 0, sizeof(resources));
> +
>      resources[0].start = cte.offset;
>      resources[0].end   = cte.offset + (cte.length - 1);
>      resources[0].flags = IORESOURCE_MEM;
> @@ -311,8 +313,8 @@ int  probe_core_uio(unsigned int core_num, struct kp2000_device *pcard, char *na
>  static int  create_dma_engine_core(struct kp2000_device *pcard, size_t engine_regs_offset, int engine_num, int irq_num)
>  {
>      struct mfd_cell  cell = {0};
> -    struct resource  resources[2] = {0};
> -
> +    struct resource  resources[2];
> +
>      dev_dbg(&pcard->pdev->dev, "create_dma_core(pcard = [%p], engine_regs_offset = %zx, engine_num = %d)\n", pcard, engine_regs_offset, engine_num);
>
>      cell.platform_data = NULL;
> @@ -321,6 +323,8 @@ static int  create_dma_engine_core(struct kp2000_device *pcard, size_t engine_re
>      cell.name = KP_DRIVER_NAME_DMA_CONTROLLER;
>      cell.num_resources = 2;
>
> +    memset(&resources, 0, sizeof(resources));
> +

Bonus points for clearing up the leading whitespace.  Thanks for the
patch.  I'm beginning to think aggregate initializers are the devil.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>      resources[0].start = engine_regs_offset;
>      resources[0].end   = engine_regs_offset + (KPC_DMA_ENGINE_SIZE - 1);
>      resources[0].flags = IORESOURCE_MEM;
> --
> 2.21.0
>


-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] staging: kpc2000: Use memset to initialize resources

[Dan Carpenter]

On Wed, Apr 24, 2019 at 11:57:43AM -0700, Nathan Chancellor wrote:
> diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> index ad2cc0a3bfa1..13f544f3c0b9 100644
> --- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
> +++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> @@ -93,8 +93,8 @@ void parse_core_table_entry(struct core_table_entry *cte, const u64 read_val, co
>  int  probe_core_basic(unsigned int core_num, struct kp2000_device *pcard, char *name, const struct core_table_entry cte)
>  {
>      struct mfd_cell  cell = {0};
> -    struct resource  resources[2] = {0};
> -    
> +    struct resource  resources[2];
> +
>      struct kpc_core_device_platdata  core_pdata = {

Greg already applied this and that's cool but I would have probably
gone with "struct resource resources[2] = {};".  memset() is only
required if we want to clear out the struct holes because we're going to
copy the whole struct to userspace.  (Some compilers will change
foo = {} into "foo.a = 0; foo.b = 0;" when it's faster than doing a
memset, so the struct holes don't always get cleared).

Also it was risky from a process perspective to delete the stray tab
from the next line because some one could have argued that it was
unrelated or that the whole line should be removed instead.  You would
have had to redo the patch for something silly... #YOLO #LivingOnTheEdge

But in this case, it's already applied so everything worked out. :)

regards,
dan carpenter

Re: [PATCH] staging: kpc2000: Use memset to initialize resources

[Nick Desaulniers]

On Tue, Apr 30, 2019 at 2:12 AM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Also it was risky from a process perspective to delete the stray tab
> from the next line because some one could have argued that it was
> unrelated or that the whole line should be removed instead.  You would
> have had to redo the patch for something silly... #YOLO #LivingOnTheEdge

Future patches to Dan shall contain the hashtag #YOLO.
-- 
Thanks,
~Nick Desaulniers