[Cocci] Replacing a struct member with a function call

[Cocci] Replacing a struct member with a function call

[Thomas Adam]

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

Re: [Cocci] Replacing a struct member with a function call

[Wolfram Sang]

[-- 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

Re: [Cocci] Replacing a struct member with a function call

[Julia Lawall]

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

Re: [Cocci] Replacing a struct member with a function call

[Thomas Adam]

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

Re: [Cocci] Replacing a struct member with a function call

[Julia Lawall]

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

Re: [Cocci] Replacing a struct member with a function call

[Mansour Moufid]

[-- 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

Re: [Cocci] Replacing a struct member with a function call

[Julia Lawall]

[-- 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

Re: [Cocci] Replacing a struct member with a function call

[Thomas Adam]

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