All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] constify char attribute_group structures
@ 2017-08-02 11:19 Arvind Yadav
  2017-08-02 11:19 ` [PATCH 1/3] char: tlclk: constify " Arvind Yadav
  2017-08-02 17:56 ` [PATCH 0/3] constify char " Gross, Mark
  0 siblings, 2 replies; 6+ messages in thread
From: Arvind Yadav @ 2017-08-02 11:19 UTC (permalink / raw)
  To: arnd, gregkh, mark.gross; +Cc: linux-kernel

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by <linux/sysfs.h> work with
const attribute_group. So mark the non-const structs as const.

Arvind Yadav (3):
  [PATCH 1/3] char: tlclk: constify attribute_group structures.
  [PATCH 2/3] char: tpm: constify attribute_group structures.
  [PATCH 3/3] char: virtio: constify attribute_group structures.

 drivers/char/tlclk.c          | 2 +-
 drivers/char/tpm/tpm_ppi.c    | 2 +-
 drivers/char/virtio_console.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/3] char: tlclk: constify attribute_group structures.
  2017-08-02 11:19 [PATCH 0/3] constify char attribute_group structures Arvind Yadav
@ 2017-08-02 11:19 ` Arvind Yadav
  2017-08-02 18:15   ` Gross, Mark
  2017-08-02 17:56 ` [PATCH 0/3] constify char " Gross, Mark
  1 sibling, 1 reply; 6+ messages in thread
From: Arvind Yadav @ 2017-08-02 11:19 UTC (permalink / raw)
  To: arnd, gregkh, mark.gross; +Cc: linux-kernel

attribute_group are not supposed to change at runtime. All functions
working with attribute_group provided by <linux/sysfs.h> work with
const attribute_group. So mark the non-const structs as const.

Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
 drivers/char/tlclk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/tlclk.c b/drivers/char/tlclk.c
index 572a517..6210bff 100644
--- a/drivers/char/tlclk.c
+++ b/drivers/char/tlclk.c
@@ -766,7 +766,7 @@ static ssize_t store_reset (struct device *d,
 	NULL
 };
 
-static struct attribute_group tlclk_attribute_group = {
+static const struct attribute_group tlclk_attribute_group = {
 	.name = NULL,		/* put in device directory */
 	.attrs = tlclk_sysfs_entries,
 };
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* RE: [PATCH 0/3] constify char attribute_group structures
  2017-08-02 11:19 [PATCH 0/3] constify char attribute_group structures Arvind Yadav
  2017-08-02 11:19 ` [PATCH 1/3] char: tlclk: constify " Arvind Yadav
@ 2017-08-02 17:56 ` Gross, Mark
  2017-08-03  5:34   ` Arvind Yadav
  1 sibling, 1 reply; 6+ messages in thread
From: Gross, Mark @ 2017-08-02 17:56 UTC (permalink / raw)
  To: Arvind Yadav, arnd, gregkh; +Cc: linux-kernel

Why stop at these 3 users of attribute_group?

--mark

-----Original Message-----
From: Arvind Yadav [mailto:arvind.yadav.cs@gmail.com] 
Sent: Wednesday, August 2, 2017 4:19 AM
To: arnd@arndb.de; gregkh@linuxfoundation.org; Gross, Mark <mark.gross@intel.com>
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH 0/3] constify char attribute_group structures

attribute_group are not supposed to change at runtime. All functions working with attribute_group provided by <linux/sysfs.h> work with const attribute_group. So mark the non-const structs as const.

Arvind Yadav (3):
  [PATCH 1/3] char: tlclk: constify attribute_group structures.
  [PATCH 2/3] char: tpm: constify attribute_group structures.
  [PATCH 3/3] char: virtio: constify attribute_group structures.

 drivers/char/tlclk.c          | 2 +-
 drivers/char/tpm/tpm_ppi.c    | 2 +-
 drivers/char/virtio_console.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

--
1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH 1/3] char: tlclk: constify attribute_group structures.
  2017-08-02 11:19 ` [PATCH 1/3] char: tlclk: constify " Arvind Yadav
@ 2017-08-02 18:15   ` Gross, Mark
  0 siblings, 0 replies; 6+ messages in thread
From: Gross, Mark @ 2017-08-02 18:15 UTC (permalink / raw)
  To: Arvind Yadav, arnd, gregkh; +Cc: linux-kernel

The driver doesn't seem to touch the name field after its defined it looks ok functionally.  

Note: I am not a fan of const declarations in the C language.  It's too complicated when we get into casting and pointers to structures.

I'm not sure there is a lot of value in being pedantic on a few users of attribute_group structures.  I'm thinking this change should be all or nothing and change core function prototypes to create warnings if they are not used with properly defined const 's.  Otherwise this is just a cosmetic change of minimal value to a subset of drivers.   

FWIW there does seem to be a good number (cscope finds 1700+) of attribute_group users across the kernel and a good number of them are not using const.  If you go down this pedantic path I don't think you should at just 3 drivers.

Reviewed-by: mark gross <mark.gross@intel.com>

--mark

> -----Original Message-----
> From: Arvind Yadav [mailto:arvind.yadav.cs@gmail.com]
> Sent: Wednesday, August 2, 2017 4:19 AM
> To: arnd@arndb.de; gregkh@linuxfoundation.org; Gross, Mark
> <mark.gross@intel.com>
> Cc: linux-kernel@vger.kernel.org
> Subject: [PATCH 1/3] char: tlclk: constify attribute_group structures.
> 
> attribute_group are not supposed to change at runtime. All functions working
> with attribute_group provided by <linux/sysfs.h> work with const
> attribute_group. So mark the non-const structs as const.
> 
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
>  drivers/char/tlclk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/char/tlclk.c b/drivers/char/tlclk.c index 572a517..6210bff
> 100644
> --- a/drivers/char/tlclk.c
> +++ b/drivers/char/tlclk.c
> @@ -766,7 +766,7 @@ static ssize_t store_reset (struct device *d,
>  	NULL
>  };
> 
> -static struct attribute_group tlclk_attribute_group = {
> +static const struct attribute_group tlclk_attribute_group = {
>  	.name = NULL,		/* put in device directory */
>  	.attrs = tlclk_sysfs_entries,
>  };
> --
> 1.9.1

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 0/3] constify char attribute_group structures
  2017-08-02 17:56 ` [PATCH 0/3] constify char " Gross, Mark
@ 2017-08-03  5:34   ` Arvind Yadav
  2017-08-03 12:37     ` Gross, Mark
  0 siblings, 1 reply; 6+ messages in thread
From: Arvind Yadav @ 2017-08-03  5:34 UTC (permalink / raw)
  To: Gross, Mark, arnd, gregkh; +Cc: linux-kernel

Hi Mark,


On Wednesday 02 August 2017 11:26 PM, Gross, Mark wrote:
> Why stop at these 3 users of attribute_group?
I am doing for all. This changes is only for char user. Few patch are
under review and few are merged. Rest all I will send.

~arvind
> --mark
>
> -----Original Message-----
> From: Arvind Yadav [mailto:arvind.yadav.cs@gmail.com]
> Sent: Wednesday, August 2, 2017 4:19 AM
> To: arnd@arndb.de; gregkh@linuxfoundation.org; Gross, Mark <mark.gross@intel.com>
> Cc: linux-kernel@vger.kernel.org
> Subject: [PATCH 0/3] constify char attribute_group structures
>
> attribute_group are not supposed to change at runtime. All functions working with attribute_group provided by <linux/sysfs.h> work with const attribute_group. So mark the non-const structs as const.
>
> Arvind Yadav (3):
>    [PATCH 1/3] char: tlclk: constify attribute_group structures.
>    [PATCH 2/3] char: tpm: constify attribute_group structures.
>    [PATCH 3/3] char: virtio: constify attribute_group structures.
>
>   drivers/char/tlclk.c          | 2 +-
>   drivers/char/tpm/tpm_ppi.c    | 2 +-
>   drivers/char/virtio_console.c | 2 +-
>   3 files changed, 3 insertions(+), 3 deletions(-)
>
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* RE: [PATCH 0/3] constify char attribute_group structures
  2017-08-03  5:34   ` Arvind Yadav
@ 2017-08-03 12:37     ` Gross, Mark
  0 siblings, 0 replies; 6+ messages in thread
From: Gross, Mark @ 2017-08-03 12:37 UTC (permalink / raw)
  To: Arvind Yadav, arnd, gregkh; +Cc: linux-kernel

Sounds good to me.
--mark

> -----Original Message-----
> From: Arvind Yadav [mailto:arvind.yadav.cs@gmail.com]
> Sent: Wednesday, August 2, 2017 10:35 PM
> To: Gross, Mark <mark.gross@intel.com>; arnd@arndb.de;
> gregkh@linuxfoundation.org
> Cc: linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 0/3] constify char attribute_group structures
> 
> Hi Mark,
> 
> 
> On Wednesday 02 August 2017 11:26 PM, Gross, Mark wrote:
> > Why stop at these 3 users of attribute_group?
> I am doing for all. This changes is only for char user. Few patch are under
> review and few are merged. Rest all I will send.
> 
> ~arvind
> > --mark
> >
> > -----Original Message-----
> > From: Arvind Yadav [mailto:arvind.yadav.cs@gmail.com]
> > Sent: Wednesday, August 2, 2017 4:19 AM
> > To: arnd@arndb.de; gregkh@linuxfoundation.org; Gross, Mark
> > <mark.gross@intel.com>
> > Cc: linux-kernel@vger.kernel.org
> > Subject: [PATCH 0/3] constify char attribute_group structures
> >
> > attribute_group are not supposed to change at runtime. All functions working
> with attribute_group provided by <linux/sysfs.h> work with const
> attribute_group. So mark the non-const structs as const.
> >
> > Arvind Yadav (3):
> >    [PATCH 1/3] char: tlclk: constify attribute_group structures.
> >    [PATCH 2/3] char: tpm: constify attribute_group structures.
> >    [PATCH 3/3] char: virtio: constify attribute_group structures.
> >
> >   drivers/char/tlclk.c          | 2 +-
> >   drivers/char/tpm/tpm_ppi.c    | 2 +-
> >   drivers/char/virtio_console.c | 2 +-
> >   3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > --
> > 1.9.1
> >

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-03 12:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-02 11:19 [PATCH 0/3] constify char attribute_group structures Arvind Yadav
2017-08-02 11:19 ` [PATCH 1/3] char: tlclk: constify " Arvind Yadav
2017-08-02 18:15   ` Gross, Mark
2017-08-02 17:56 ` [PATCH 0/3] constify char " Gross, Mark
2017-08-03  5:34   ` Arvind Yadav
2017-08-03 12:37     ` Gross, Mark

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.