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

Groups > linux.kernel > #1705336 > unrolled thread

[PATCH] ata: make ata_port_operations const

Back to article view | Back to linux.kernel

Contents

  [PATCH] ata: make ata_port_operations const Bhumika Goyal <bhumirks@gmail.com> - 2017-08-07 12:40 +0200
    Re: [PATCH] ata: make ata_port_operations const Tejun Heo <tj@kernel.org> - 2017-08-07 17:10 +0200
      Re: [PATCH] ata: make ata_port_operations const Bhumika Goyal <bhumirks@gmail.com> - 2017-08-07 17:30 +0200
        Re: [PATCH] ata: make ata_port_operations const Tejun Heo <tj@kernel.org> - 2017-08-07 18:30 +0200
          Re: [PATCH] ata: make ata_port_operations const Joe Perches <joe@perches.com> - 2017-08-07 18:40 +0200
            Re: [PATCH] ata: make ata_port_operations const Bhumika Goyal <bhumirks@gmail.com> - 2017-08-08 10:20 +0200

#1705336 — [PATCH] ata: make ata_port_operations const

Make ata_port_operations structures const as it is only stored in the
inherits field of an ata_port_operations structure. Therefore make it
const.

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 drivers/ata/pata_sis.c | 2 +-
 drivers/ata/sata_via.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/pata_sis.c b/drivers/ata/pata_sis.c
index 626f989..24614d2 100644
--- a/drivers/ata/pata_sis.c
+++ b/drivers/ata/pata_sis.c
@@ -549,7 +549,7 @@ static unsigned long sis_133_mode_filter(struct ata_device *adev, unsigned long
 	.cable_detect		= sis_133_cable_detect,
 };
 
-static struct ata_port_operations sis_base_ops = {
+static const struct ata_port_operations sis_base_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 	.prereset		= sis_pre_reset,
 };
diff --git a/drivers/ata/sata_via.c b/drivers/ata/sata_via.c
index 93b8d78..83ba848 100644
--- a/drivers/ata/sata_via.c
+++ b/drivers/ata/sata_via.c
@@ -129,7 +129,7 @@ struct svia_priv {
 	ATA_BMDMA_SHT(DRV_NAME),
 };
 
-static struct ata_port_operations svia_base_ops = {
+static const  struct ata_port_operations svia_base_ops = {
 	.inherits		= &ata_bmdma_port_ops,
 	.sff_tf_load		= svia_tf_load,
 };
-- 
1.9.1

[toc] | [next] | [standalone]

#1705621

Hello,

On Mon, Aug 07, 2017 at 04:02:02PM +0530, Bhumika Goyal wrote:
> Make ata_port_operations structures const as it is only stored in the
> inherits field of an ata_port_operations structure. Therefore make it
> const.
> 
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>

How did you test the patch?

Thanks.

-- 
tejun

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

#1705642

On Mon, Aug 7, 2017 at 8:37 PM, Tejun Heo <tj@kernel.org> wrote:
> Hello,
>
> On Mon, Aug 07, 2017 at 04:02:02PM +0530, Bhumika Goyal wrote:
>> Make ata_port_operations structures const as it is only stored in the
>> inherits field of an ata_port_operations structure. Therefore make it
>> const.
>>
>> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
>
> How did you test the patch?
>

I compiled the .o files and checked the size before and after
compilation. After making the structure const, bytes should move from
data to text segment of the memory and this is what happened. So, this
is how I tested it.

Thanks,
Bhumika

> Thanks.
>
> --
> tejun

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

#1705695

Hello,

On Mon, Aug 07, 2017 at 08:54:21PM +0530, Bhumika Goyal wrote:
> I compiled the .o files and checked the size before and after
> compilation. After making the structure const, bytes should move from
> data to text segment of the memory and this is what happened. So, this
> is how I tested it.

Ah, I see, so the port_operations is never used directly.  I'm not
sure whether it'd be a good idea to selectively pick these and makr
them const.  Let's just leave them be.

Thanks.

-- 
tejun

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

#1705702

On Mon, 2017-08-07 at 09:29 -0700, Tejun Heo wrote:
> Hello,
> 
> On Mon, Aug 07, 2017 at 08:54:21PM +0530, Bhumika Goyal wrote:
> > I compiled the .o files and checked the size before and after
> > compilation. After making the structure const, bytes should move from
> > data to text segment of the memory and this is what happened. So, this
> > is how I tested it.
> 
> Ah, I see, so the port_operations is never used directly.  I'm not
> sure whether it'd be a good idea to selectively pick these and makr
> them const.  Let's just leave them be.

Why?

Things that are never accessed as other than const should
be const no?

Why leave unnecessary exposure for muckery?

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

#1706139

On Mon, Aug 7, 2017 at 10:04 PM, Joe Perches <joe@perches.com> wrote:
> On Mon, 2017-08-07 at 09:29 -0700, Tejun Heo wrote:
>> Hello,
>>
>> On Mon, Aug 07, 2017 at 08:54:21PM +0530, Bhumika Goyal wrote:
>> > I compiled the .o files and checked the size before and after
>> > compilation. After making the structure const, bytes should move from
>> > data to text segment of the memory and this is what happened. So, this
>> > is how I tested it.
>>
>> Ah, I see, so the port_operations is never used directly.  I'm not
>> sure whether it'd be a good idea to selectively pick these and makr
>> them const.  Let's just leave them be.
>
> Why?
>
> Things that are never accessed as other than const should
> be const no?
>

I think it would be better to leave them as in most situations the
ata_port_operations structures cannot be const as they are only used
in the port_ops field of an ata_port_info structure and this field is
not const. Having two ata_port_operations const and others non-const
will cause code inconsistency.

Thanks,
Bhumika

> Why leave unnecessary exposure for muckery?

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web