Coccinelle archive on lore.kernel.org
 help / color / Atom feed
* [Cocci] Replacing the type casting
@ 2020-02-08 13:48 Ondřej Surý
  2020-02-08 16:07 ` Julia Lawall
  0 siblings, 1 reply; 6+ messages in thread
From: Ondřej Surý @ 2020-02-08 13:48 UTC (permalink / raw)
  To: cocci

Hi,

I have a code like this:

```
static inline void
freestruct_in_nsap(ARGS_FREESTRUCT) {
        dns_rdata_in_nsap_t *nsap;

        REQUIRE(((dns_rdata_in_nsap_t *)source) != NULL);
        REQUIRE(((dns_rdata_in_nsap_t *)source)->common.rdclass == dns_rdataclass_in);
        REQUIRE(((dns_rdata_in_nsap_t *)source)->common.rdtype == dns_rdatatype_nsap);

        nsap = source;
```

and due to various combination of a C standard requirement and enforced cppcheck rules, I can’t change it to a saner workflow for a legacy code.

But I am trying to replace the specific type case with a generic structure, so it will look like this:

```
static inline void
freestruct_in_nsap(ARGS_FREESTRUCT) {
        dns_rdata_in_nsap_t *nsap;

        REQUIRE(source != NULL);
        REQUIRE(((dns_rdatacommon_t *)source)->rdclass == dns_rdataclass_in);
        REQUIRE(((dns_rdatacommon_t *)source)->rdtype == dns_rdatatype_nsap);

        nsap = source;

```

These are my rules currently:

```
@@
type T;
expression source;
@@

- REQUIRE(((T *)source) != NULL);
+ REQUIRE(source != NULL);

@@
type T;
type R;
identifier common, rdtype;
T *target;
expression source;
@@

  REQUIRE(source != NULL);
  ...
  REQUIRE((
- (T *)
+ (dns_rdatacommon_t *)
  source)->
- common.
  rdtype == ...);
  <+...
  target = source;
  ...+>
```

the first one works like a charm, but the second rule fails with:

```
init_defs_builtins: /usr/local/bin/../Cellar/coccinelle/1.0.8/bin/../lib/coccinelle/standard.h
plus: parse error:
  File "cocci/rdata.spatch", line 21, column 22, charpos = 251
  around = ')',
  whole content = + (dns_rdatacommon_t *)
```

I must be missing something very obvious as just removing the type works, but adding the `+ (dns_rdatacommon_t *)` line makes the rule fail.

Thanks,
Ondrej
--
Ondřej Surý
ondrej@sury.org



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

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

* Re: [Cocci] Replacing the type casting
  2020-02-08 13:48 [Cocci] Replacing the type casting Ondřej Surý
@ 2020-02-08 16:07 ` Julia Lawall
  0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2020-02-08 16:07 UTC (permalink / raw)
  To: Ondřej Surý; +Cc: cocci

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



On Sat, 8 Feb 2020, Ondřej Surý wrote:

> Hi,
>
> I have a code like this:
>
> ```
> static inline void
> freestruct_in_nsap(ARGS_FREESTRUCT) {
>         dns_rdata_in_nsap_t *nsap;
>
>         REQUIRE(((dns_rdata_in_nsap_t *)source) != NULL);
>         REQUIRE(((dns_rdata_in_nsap_t *)source)->common.rdclass == dns_rdataclass_in);
>         REQUIRE(((dns_rdata_in_nsap_t *)source)->common.rdtype == dns_rdatatype_nsap);
>
>         nsap = source;
> ```
>
> and due to various combination of a C standard requirement and enforced cppcheck rules, I can’t change it to a saner workflow for a legacy code.
>
> But I am trying to replace the specific type case with a generic structure, so it will look like this:
>
> ```
> static inline void
> freestruct_in_nsap(ARGS_FREESTRUCT) {
>         dns_rdata_in_nsap_t *nsap;
>
>         REQUIRE(source != NULL);
>         REQUIRE(((dns_rdatacommon_t *)source)->rdclass == dns_rdataclass_in);
>         REQUIRE(((dns_rdatacommon_t *)source)->rdtype == dns_rdatatype_nsap);
>
>         nsap = source;
>
> ```
>
> These are my rules currently:
>
> ```
> @@
> type T;
> expression source;
> @@
>
> - REQUIRE(((T *)source) != NULL);
> + REQUIRE(source != NULL);
>
> @@
> type T;
> type R;
> identifier common, rdtype;
> T *target;
> expression source;
> @@
>
>   REQUIRE(source != NULL);
>   ...
>   REQUIRE((
> - (T *)
> + (dns_rdatacommon_t *)
>   source)->
> - common.
>   rdtype == ...);
>   <+...
>   target = source;
>   ...+>
> ```
>
> the first one works like a charm, but the second rule fails with:
>
> ```
> init_defs_builtins: /usr/local/bin/../Cellar/coccinelle/1.0.8/bin/../lib/coccinelle/standard.h
> plus: parse error:
>   File "cocci/rdata.spatch", line 21, column 22, charpos = 251
>   around = ')',
>   whole content = + (dns_rdatacommon_t *)
> ```
>
> I must be missing something very obvious as just removing the type works, but adding the `+ (dns_rdatacommon_t *)` line makes the rule fail.

Coccinelle is not able to infer typedefs in casts.  So you should add to
your list of metavariables the declaration:

typedef dns_rdatacommon_t;

julia


>
> Thanks,
> Ondrej
> --
> Ondřej Surý
> ondrej@sury.org
>
>
>
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- 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] 6+ messages in thread

* Re: [Cocci] Replacing the type casting
  2020-02-08 20:09 ` Ondřej Surý
@ 2020-02-08 20:37   ` Markus Elfring
  0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-02-08 20:37 UTC (permalink / raw)
  To: Ondřej Surý; +Cc: cocci

>> REQUIRE((
>> -         (T*)
>>          source) != NULL);
>
> Thanks, that’s helpful.

Thanks for such a view on the suggested transformation approach.


>>>  REQUIRE((
>>> - (T *)
>>> + (dns_rdatacommon_t *)
>>
>> (
>> -T
>> +dns_rdatacommon_t
>> *)
>
> This cannot be done,

I propose to take another look at how many adjustments such a type cast
will need actually.
Will the replacement of the data type be sufficient at this place
of SmPL code?


> as I need to convert only .common members, …

This concern can be added to the referenced change pattern.

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

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

* Re: [Cocci] Replacing the type casting
  2020-02-08 19:44 Markus Elfring
@ 2020-02-08 20:09 ` Ondřej Surý
  2020-02-08 20:37   ` Markus Elfring
  0 siblings, 1 reply; 6+ messages in thread
From: Ondřej Surý @ 2020-02-08 20:09 UTC (permalink / raw)
  To: Markus Elfring; +Cc: cocci

Hi Markus,

I ended up with:

```
@@
type T;
expression source;
@@

- REQUIRE(((T *)source) != NULL);
+ REQUIRE(source != NULL);


@@
type T;
typedef dns_rdatacommon_t;
identifier common, rdtype;
identifier source;
@@

  REQUIRE((
- (T *)source
+ (dns_rdatacommon_t *)source
  )->
- common.
  rdtype == ...);

@@
type T;
typedef dns_rdatacommon_t;
identifier common, rdclass;
identifier source;
@@

  REQUIRE((
- (T *)source
+ (dns_rdatacommon_t *)source
  )->
- common.
  rdclass == ...);
```


> On 8 Feb 2020, at 11:44, Markus Elfring <Markus.Elfring@web.de> wrote:
> 
> Can the following transformation variants be also helpful?
> 
> 
>> @@
>> type T;
>> expression source;
>> @@
>> 
>> - REQUIRE(((T *)source) != NULL);
>> + REQUIRE(source != NULL);
> 
> REQUIRE((
> -         (T*)
>          source) != NULL);

Thanks, that’s helpful. I’ll use that.

>> @@
>> 
>>  REQUIRE(source != NULL);
>>  ...
>>  REQUIRE((
>> - (T *)
>> + (dns_rdatacommon_t *)
> 
> (
> -T
> +dns_rdatacommon_t
> *)


This cannot be done, as I need to convert only .common members, as the more specific data types include dns_rdatacommon_t as a first member of the struct (a header), but they usually contain more fields, f.e.:

        REQUIRE(type == dns_rdatatype_nsec);
        REQUIRE((source) != NULL);
        REQUIRE(((dns_rdatacommon_t *)source)->rdtype == type);
        REQUIRE(((dns_rdatacommon_t *)source)->rdclass == rdclass);
        REQUIRE(((dns_rdata_nsec_t *)source)->typebits != NULL ||
                ((dns_rdata_nsec_t *)source)->len == 0);

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

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

* Re: [Cocci] Replacing the type casting
@ 2020-02-08 19:44 Markus Elfring
  2020-02-08 20:09 ` Ondřej Surý
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2020-02-08 19:44 UTC (permalink / raw)
  To: Ondřej Surý; +Cc: cocci

Can the following transformation variants be also helpful?


> @@
> type T;
> expression source;
> @@
>
> - REQUIRE(((T *)source) != NULL);
> + REQUIRE(source != NULL);

 REQUIRE((
-         (T*)
          source) != NULL);


> @@
> type T;
> type R;

type T;


> identifier common, rdtype;
> T *target;
> expression source;

typedef dns_rdatacommon_t;


> @@
>
>   REQUIRE(source != NULL);
>   ...
>   REQUIRE((
> - (T *)
> + (dns_rdatacommon_t *)

 (
-T
+dns_rdatacommon_t
 *)


Will any constraints become relevant for the deleted data types?

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

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

* Re: [Cocci] Replacing the type casting
@ 2020-02-08 19:06 Markus Elfring
  0 siblings, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2020-02-08 19:06 UTC (permalink / raw)
  To: Julia Lawall, cocci

> > I must be missing something very obvious as just removing the type works, but adding the `+ (dns_rdatacommon_t *)` line makes the rule fail.
>
> Coccinelle is not able to infer typedefs in casts.

Under which circumstances can this software situation be reconsidered?

How long will an extra metavariable specification like “typedef dns_rdatacommon_t;”
be needed for the mentioned transformation approach?

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

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

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-08 13:48 [Cocci] Replacing the type casting Ondřej Surý
2020-02-08 16:07 ` Julia Lawall
2020-02-08 19:06 Markus Elfring
2020-02-08 19:44 Markus Elfring
2020-02-08 20:09 ` Ondřej Surý
2020-02-08 20:37   ` Markus Elfring

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