Coccinelle Archive on lore.kernel.org
 help / color / Atom feed
* [Cocci] Replacing a struct member with a function call
@ 2021-03-14  0:55 Thomas Adam
  2021-03-14  6:10 ` Wolfram Sang
  0 siblings, 1 reply; 8+ messages in thread
From: Thomas Adam @ 2021-03-14  0:55 UTC (permalink / raw)
  To: Coccinelle

Hello all,

I'm wanting to replace a struct member with a function call across the
code base I'm working on.

For example, I currently have the following struct definition (simplified):

struct monitor {
    struct {
        int width;
        int height
    } virtual;
};

Across my code base, I typically have the following:

struct monitor *m;
int f = m->virtual.width;

I have a need to remove `m->virtual.width` with a function call, hence
the example above after the modified code would look something like:

struct monitor *m;
int f = get_monitor_width();

I've tried the following smPL code without any success.  Could someone
point me in the right direction?  TIA!

@@
type M;
@@
- m->virtual.width;
+ get_monitor_width();

-- Thomas
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Cocci] Replacing a struct member with a function call
  2021-03-14  0:55 [Cocci] Replacing a struct member with a function call Thomas Adam
@ 2021-03-14  6:10 ` Wolfram Sang
  2021-03-14  9:16   ` Julia Lawall
  0 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2021-03-14  6:10 UTC (permalink / raw)
  To: Thomas Adam; +Cc: Coccinelle

[-- Attachment #1.1: Type: text/plain, Size: 96 bytes --]


> @@
> type M;

This?

struct monitor* m;

> @@
> - m->virtual.width;
> + get_monitor_width();

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Cocci] Replacing a struct member with a function call
  2021-03-14  6:10 ` Wolfram Sang
@ 2021-03-14  9:16   ` Julia Lawall
  2021-03-15  0:43     ` Thomas Adam
  0 siblings, 1 reply; 8+ messages in thread
From: Julia Lawall @ 2021-03-14  9:16 UTC (permalink / raw)
  To: Wolfram Sang; +Cc: Coccinelle



On Sun, 14 Mar 2021, Wolfram Sang wrote:

>
> > @@
> > type M;
>
> This?
>
> struct monitor* m;
>
> > @@
> > - m->virtual.width;
> > + get_monitor_width();

I guess that m should be somewhere in teh call to get_monitor_width too?

julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Cocci] Replacing a struct member with a function call
  2021-03-14  9:16   ` Julia Lawall
@ 2021-03-15  0:43     ` Thomas Adam
  2021-03-15  6:38       ` Julia Lawall
  2021-03-16  2:48       ` Mansour Moufid
  0 siblings, 2 replies; 8+ messages in thread
From: Thomas Adam @ 2021-03-15  0:43 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Wolfram Sang, Coccinelle

Hello,

I can see I was as clear as mud with my explanation -- apologies for
that, so let me try again.

In my original example:

struct monitor {
    struct {
        int width;
        int height
    } virtual;
};

... the members width and height aren't required any more, as they're
actually computable generically, and don't belong in that struct.
Instead, I have separate functions which can provide those values.

So where I have in code, statements such as:

struct monitor *m = this_monitor();
int foo = m->virutal.width;

I want to be able to substitute "m->virtual.width" with a function
call "get_width()" -- which does not involve "struct monitor" at all.
Indeed, the semantic patch I'm trying to apply now looks like this:

@@
struct monitor *m;
@@

- m->virtual.width;
+ get_width();

... and although spatch doesn't tell me of any errors, when I run it
over my codebase, no modifications are made.  So clearly I'm still
doing something wrong.  At the point, I have some questions:

1.  Given the inner struct "virtual" inside of "struct monitor", is it
correct that spatch would understand:

m->virtual.width;

... or do I need to declare "virtual" as some expression or
identifier?  I did try:

@@
struct monitor *m;
expression virtual;
@@

- m->virtual.width;
+ get_width();

... but this results in an error.

2.  Do I need to declare "virtual" as a struct in its own right
somehow?  If so, it's not immediately obvious if this should be the
case or how to do it.

I hope I'm making some sense here -- apologies if not, and if I need
to expand upon anything further, do please let me know.

Essentially, it seems to me as though the inner struct "virtual" isn't
being declared as a valid type which spatch is understanding, and this
is why things aren't working how I'd like.

Again, thanks ever so much for you time -- everyone's been very
helpful to me in the past, and I've found coccinelle to be invaluable
to making sweeping code changes, as well as bug-fixes on my codebase,
so thanks to everyone involved in this project.  It's invaluable!

Kindly,
Thomas

On Sun, 14 Mar 2021 at 09:16, Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>
> On Sun, 14 Mar 2021, Wolfram Sang wrote:
>
> >
> > > @@
> > > type M;
> >
> > This?
> >
> > struct monitor* m;
> >
> > > @@
> > > - m->virtual.width;
> > > + get_monitor_width();
>
> I guess that m should be somewhere in teh call to get_monitor_width too?
>
> julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Cocci] Replacing a struct member with a function call
  2021-03-15  0:43     ` Thomas Adam
@ 2021-03-15  6:38       ` Julia Lawall
  2021-03-16  2:48       ` Mansour Moufid
  1 sibling, 0 replies; 8+ messages in thread
From: Julia Lawall @ 2021-03-15  6:38 UTC (permalink / raw)
  To: Thomas Adam; +Cc: Wolfram Sang, Coccinelle



On Mon, 15 Mar 2021, Thomas Adam wrote:

> Hello,
>
> I can see I was as clear as mud with my explanation -- apologies for
> that, so let me try again.
>
> In my original example:
>
> struct monitor {
>     struct {
>         int width;
>         int height
>     } virtual;
> };
>
> ... the members width and height aren't required any more, as they're
> actually computable generically, and don't belong in that struct.
> Instead, I have separate functions which can provide those values.
>
> So where I have in code, statements such as:
>
> struct monitor *m = this_monitor();
> int foo = m->virutal.width;
>
> I want to be able to substitute "m->virtual.width" with a function
> call "get_width()" -- which does not involve "struct monitor" at all.
> Indeed, the semantic patch I'm trying to apply now looks like this:
>
> @@
> struct monitor *m;
> @@
>
> - m->virtual.width;
> + get_width();
>
> ... and although spatch doesn't tell me of any errors, when I run it
> over my codebase, no modifications are made.  So clearly I'm still
> doing something wrong.  At the point, I have some questions:
>
> 1.  Given the inner struct "virtual" inside of "struct monitor", is it
> correct that spatch would understand:
>
> m->virtual.width;
>
> ... or do I need to declare "virtual" as some expression or
> identifier?  I did try:
>
> @@
> struct monitor *m;
> expression virtual;
> @@
>
> - m->virtual.width;
> + get_width();
>
> ... but this results in an error.
>
> 2.  Do I need to declare "virtual" as a struct in its own right
> somehow?  If so, it's not immediately obvious if this should be the
> case or how to do it.
>
> I hope I'm making some sense here -- apologies if not, and if I need
> to expand upon anything further, do please let me know.
>
> Essentially, it seems to me as though the inner struct "virtual" isn't
> being declared as a valid type which spatch is understanding, and this
> is why things aren't working how I'd like.
>
> Again, thanks ever so much for you time -- everyone's been very
> helpful to me in the past, and I've found coccinelle to be invaluable
> to making sweeping code changes, as well as bug-fixes on my codebase,
> so thanks to everyone involved in this project.  It's invaluable!

spatch doens't care about virtual.  Identifier is fine.  If the code is
really as simple as you have presented it, ie with the type of m declared
immediately before its use, then there may be a problem with parsing the
code.  Run spatch --parse-c on a file that you think should be changed and
see if BAD or bad come out in front of the code that should be
changed.nYOu can also run spatch --parse-c on the entire directory (may
require removing any limits on the stack size).  That will conclude by
telling you the tokens it is most unhappy with.  You can often get away
with defining a few dummy macros.

If none of that works, please send a small function that illustrates the
problem and the complete semantic patch.

julia



>
> Kindly,
> Thomas
>
> On Sun, 14 Mar 2021 at 09:16, Julia Lawall <julia.lawall@inria.fr> wrote:
> >
> >
> >
> > On Sun, 14 Mar 2021, Wolfram Sang wrote:
> >
> > >
> > > > @@
> > > > type M;
> > >
> > > This?
> > >
> > > struct monitor* m;
> > >
> > > > @@
> > > > - m->virtual.width;
> > > > + get_monitor_width();
> >
> > I guess that m should be somewhere in teh call to get_monitor_width too?
> >
> > julia
>
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Cocci] Replacing a struct member with a function call
  2021-03-15  0:43     ` Thomas Adam
  2021-03-15  6:38       ` Julia Lawall
@ 2021-03-16  2:48       ` Mansour Moufid
  2021-03-16  7:20         ` Julia Lawall
  1 sibling, 1 reply; 8+ messages in thread
From: Mansour Moufid @ 2021-03-16  2:48 UTC (permalink / raw)
  To: Thomas Adam; +Cc: Coccinelle

[-- Attachment #1.1: Type: text/plain, Size: 1120 bytes --]

On Sun, Mar 14, 2021, 20:43 Thomas Adam <thomas@xteddy.org> wrote:

> Hello,
>
> I can see I was as clear as mud with my explanation -- apologies for
> that, so let me try again.
>
> In my original example:
>
> struct monitor {
>     struct {
>         int width;
>         int height
>     } virtual;
> };
>
> ... the members width and height aren't required any more, as they're
> actually computable generically, and don't belong in that struct.
> Instead, I have separate functions which can provide those values.
>
> So where I have in code, statements such as:
>
> struct monitor *m = this_monitor();
> int foo = m->virutal.width;
>
> I want to be able to substitute "m->virtual.width" with a function
> call "get_width()" -- which does not involve "struct monitor" at all.
> Indeed, the semantic patch I'm trying to apply now looks like this:
>
> @@
> struct monitor *m;
> @@
>
> - m->virtual.width;
> + get_width();
>
> ... and although spatch doesn't tell me of any errors, when I run it
> over my codebase, no modifications are made.  So clearly I'm still
> doing something wrong.


Remove the semi-colons. ;)

[-- Attachment #1.2: Type: text/html, Size: 1610 bytes --]

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Cocci] Replacing a struct member with a function call
  2021-03-16  2:48       ` Mansour Moufid
@ 2021-03-16  7:20         ` Julia Lawall
  2021-03-18 18:19           ` Thomas Adam
  0 siblings, 1 reply; 8+ messages in thread
From: Julia Lawall @ 2021-03-16  7:20 UTC (permalink / raw)
  To: Mansour Moufid; +Cc: Coccinelle


[-- Attachment #1: Type: text/plain, Size: 1439 bytes --]



On Mon, 15 Mar 2021, Mansour Moufid wrote:

> On Sun, Mar 14, 2021, 20:43 Thomas Adam <thomas@xteddy.org> wrote:
>       Hello,
>
>       I can see I was as clear as mud with my explanation -- apologies
>       for
>       that, so let me try again.
>
>       In my original example:
>
>       struct monitor {
>           struct {
>               int width;
>               int height
>           } virtual;
>       };
>
>       ... the members width and height aren't required any more, as
>       they're
>       actually computable generically, and don't belong in that
>       struct.
>       Instead, I have separate functions which can provide those
>       values.
>
>       So where I have in code, statements such as:
>
>       struct monitor *m = this_monitor();
>       int foo = m->virutal.width;
>
>       I want to be able to substitute "m->virtual.width" with a
>       function
>       call "get_width()" -- which does not involve "struct monitor" at
>       all.
>       Indeed, the semantic patch I'm trying to apply now looks like
>       this:
>
>       @@
>       struct monitor *m;
>       @@
>
>       - m->virtual.width;
>       + get_width();
>
>       ... and although spatch doesn't tell me of any errors, when I
>       run it
>       over my codebase, no modifications are made.  So clearly I'm
>       still
>       doing something wrong.
>
>
> Remove the semi-colons. ;)

Good catch :)

julia

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Cocci] Replacing a struct member with a function call
  2021-03-16  7:20         ` Julia Lawall
@ 2021-03-18 18:19           ` Thomas Adam
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Adam @ 2021-03-18 18:19 UTC (permalink / raw)
  To: Julia Lawall; +Cc: Coccinelle

Hi all,

Thanks for your help.  This is now resolved!

Kindly,
Thomas

On Tue, 16 Mar 2021 at 07:20, Julia Lawall <julia.lawall@inria.fr> wrote:
>
>
>
> On Mon, 15 Mar 2021, Mansour Moufid wrote:
>
> > On Sun, Mar 14, 2021, 20:43 Thomas Adam <thomas@xteddy.org> wrote:
> >       Hello,
> >
> >       I can see I was as clear as mud with my explanation -- apologies
> >       for
> >       that, so let me try again.
> >
> >       In my original example:
> >
> >       struct monitor {
> >           struct {
> >               int width;
> >               int height
> >           } virtual;
> >       };
> >
> >       ... the members width and height aren't required any more, as
> >       they're
> >       actually computable generically, and don't belong in that
> >       struct.
> >       Instead, I have separate functions which can provide those
> >       values.
> >
> >       So where I have in code, statements such as:
> >
> >       struct monitor *m = this_monitor();
> >       int foo = m->virutal.width;
> >
> >       I want to be able to substitute "m->virtual.width" with a
> >       function
> >       call "get_width()" -- which does not involve "struct monitor" at
> >       all.
> >       Indeed, the semantic patch I'm trying to apply now looks like
> >       this:
> >
> >       @@
> >       struct monitor *m;
> >       @@
> >
> >       - m->virtual.width;
> >       + get_width();
> >
> >       ... and although spatch doesn't tell me of any errors, when I
> >       run it
> >       over my codebase, no modifications are made.  So clearly I'm
> >       still
> >       doing something wrong.
> >
> >
> > Remove the semi-colons. ;)
>
> Good catch :)
>
> julia
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-14  0:55 [Cocci] Replacing a struct member with a function call Thomas Adam
2021-03-14  6:10 ` Wolfram Sang
2021-03-14  9:16   ` Julia Lawall
2021-03-15  0:43     ` Thomas Adam
2021-03-15  6:38       ` Julia Lawall
2021-03-16  2:48       ` Mansour Moufid
2021-03-16  7:20         ` Julia Lawall
2021-03-18 18:19           ` Thomas Adam

Coccinelle Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/cocci/0 cocci/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 cocci cocci/ https://lore.kernel.org/cocci \
		cocci@systeme.lip6.fr
	public-inbox-index cocci

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/fr.lip6.systeme.cocci


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git