Re: [Cocci] Matching format strings

2017-07-11 Thread Ondřej Kuzník
On Tue, Jul 11, 2017 at 11:05:54PM +0100, Ondřej Kuzník wrote:
> Yes, it removes the sprintfs, but I would have expected it to produce
> this patch (note the if()s have been removed) as it manages in other
> files:
> 
> --- servers/slapd/back-sql/add.c
> +++ /tmp/cocci-output-22443-aee224-add.c
> @@ -858,12 +858,10 @@ backsql_add_attr(
>  
>  #ifdef LDAP_DEBUG
> -   if ( LogTest( LDAP_DEBUG_TRACE ) ) {
> -   snprintf( logbuf, sizeof( logbuf ), "val[%lu], id=" 
> BACKSQL_IDNUMFMT,
> -   i, new_keyval );
> -   Debug( LDAP_DEBUG_TRACE, "   
> backsql_add_attr(\"%s\"): "
> -   "executing \"%s\" %s\n", 
> -   op->ora_e->e_name.bv_val,
> -   at_rec->bam_add_proc, logbuf );
> -   }
> +   Debug(LDAP_DEBUG_TRACE,
> + "   backsql_add_attr(\"%s\"): " "executing \"%s\" 
> val[%lu], id=" BACKSQL_IDNUMFMT\n",

Oh, somehow the end of the format string gets mangled, presumably in the
Python code, which doesn't expect this at all, so I guess this would be
the reason the last part of the patch never applies. Sorry.

> + op->ora_e->e_name.bv_val, at_rec->bam_add_proc,
> + i, new_keyval);
>  #endif
> rc = SQLExecute( sth );
> @@ -1387,14 +1385,11 @@ backsql_add( Operation *op, SlapReply *r
> }
>  
> -   if ( LogTest( LDAP_DEBUG_TRACE ) ) {
> -   char buf[ SLAP_TEXT_BUFLEN ];
> -
> -   snprintf( buf, sizeof(buf),
> -   "executing \"%s\" for dn=\"%s\"  oc_map_id=" 
> BACKSQL_IDNUMFMT " p_id=" BACKSQL_IDFMT " keyval=" BACKSQL_IDNUMFMT,
> -   bi->sql_insentry_stmt, op->ora_e->e_name.bv_val,
> -   oc->bom_id, BACKSQL_IDARG(bsi.bsi_base_id.eid_id),
> -   new_keyval );
> -   Debug( LDAP_DEBUG_TRACE, "   backsql_add(): %s\n", buf);
> -   }
> +   Debug(LDAP_DEBUG_TRACE,
> + "   backsql_add(): executing \"%s\" for dn=\"%s\"  oc_map_id=" 
> BACKSQL_IDNUMFMT " p_id=" BACKSQL_IDFMT " keyval=" BACKSQL_IDNUMFMT\n",
> + bi->sql_insentry_stmt, op->ora_e->e_name.bv_val,
> + oc->bom_id, BACKSQL_IDARG(bsi.bsi_base_id.eid_id),
> + new_keyval);
>  
> rc = SQLExecute( sth );
> 

-- 
Ondřej Kuzník
Senior Software Engineer
Symas Corporation   http://www.symas.com
Packaged, certified, and supported LDAP solutions powered by OpenLDAP
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci


Re: [Cocci] Matching format strings

2017-07-11 Thread Ondřej Kuzník
On Tue, Jul 11, 2017 at 03:10:36PM +0200, Julia Lawall wrote:
> On Tue, 11 Jul 2017, Ondřej Kuzník wrote:
>> On Mon, Jul 10, 2017 at 07:26:36PM +0200, Julia Lawall wrote:
>>> Try running spatch --parse-c on your directory to get a list at the end of
>>> the top 10 things it doesn't like.
>>
>> One of the system dependent macros SLAP_EVENT_DECL is listed in each of
>> the 10 things. The macro is used in
>> https://github.com/openldap/openldap/blob/master/servers/slapd/daemon.c#L2343
>> to declare some variables used in the corresponding event loop.
>>
>> The macro definition looks like this:
>>
>> # define SLAP_EVENT_DECL struct pollfd *revents
>>
>> or:
>>
>> # define SLAP_EVENT_DECL fd_set readfds, writefds; char *rflags
>>
>> Ignoring this macro is OK I guess, if there is a way to do that with
>> coccinelle?
> 
> Just
> 
> #define SLAP_EVENT_DECL
> 
> in the .h file. But there will be stray ;s then.  So you might want to put
> the whole first definition.

It does seem to improve things, thanks. The line 2496 still doesn't
apply even when I declare SLAP_EVENT_FNAME to something like "name" in
-macro-file-builtins and I think there were similar ones in other files
later. 

>> Also, moving onto the other patch, I get similar inconsistencies with
>> ./servers/slapd/back-sql/add.c, --parse-c doesn't list any problematic
>> statements but the output logs a few parse errors that don't make much
>> sense to me.
> 
> Could you send the complete semantic patch that you are currently
> considering?  Then I can take a look.

Attaching the patches I'm currently working with:
(-macro-file-builtins macros.h)
1. variadic.cocci
2. shortcut.cocci
3. merge.cocci

The back-sql issue above is when trying to apply 2. (shortcut.cocci).
But the one I pasted in my last email is simple enough to illustrate the
issue (trying to merge snprintf and Debug, then get rid of extra if()s)
even without the python stuff.

Thanks,

-- 
Ondřej Kuzník
Senior Software Engineer
Symas Corporation   http://www.symas.com
Packaged, certified, and supported LDAP solutions powered by OpenLDAP
#define LDAP_PF_LOCAL_SENDMSG_ARG(x)
#define LDAP_P(x) x
#define SLAP_EVENT_DECL
#define SLAP_EVENT_FNAME
@@
identifier Logs =~ "Log[0-9]";
@@
-Logs
+Log

//identifier StatslogTest =~ "StatslogTest"; 
@@
@@
-StatslogTest
+LogTest

@@
format list[2] fmt;
expression list[2] args;
expression E;
@@

Debug( E, "%@fmt@", args
-, 0
 );

@@
format list[1] fmt;
expression list[1] args;
expression E;
@@

Debug( E, "%@fmt@", args
-, 0, 0
 );

@@
expression E, S;
@@

Debug( E, S
-, 0, 0, 0
 );

@@
format list[5] fmt;
expression list[5] args;
expression E;
@@

-Statslog
+Debug
 ( E, "%@fmt@", args );

@@
format list[4] fmt;
expression list[4] args;
expression E;
@@

-Statslog
+Debug
 ( E, "%@fmt@", args
-, 0
 );

@@
format list[3] fmt;
expression list[3] args;
expression E;
@@

-Statslog
+Debug
 ( E, "%@fmt@", args
-, 0, 0
 );

@@
format list[2] fmt;
expression list[2] args;
expression E;
@@

-Statslog
+Debug
 ( E, "%@fmt@", args
-, 0, 0, 0
 );

@@
format list[1] fmt;
expression list[1] args;
expression E;
@@

-Statslog
+Debug
 ( E, "%@fmt@", args
-, 0, 0, 0, 0
 );

@@
expression E, S;
@@

-Statslog
+Debug
 ( E, S
-, 0, 0, 0, 0, 0
 );

@@
identifier Stats =~ "^Statslog";
@@
(
 StatslogEtime
|
-Stats
+Debug
)
@initialize:python@
@@

#regex from 
https://stackoverflow.com/questions/30011379/how-can-i-parse-a-c-format-string-in-python
import re
fmtstring = '''\
(  # start of capture group 1
%  # literal "%"
(?:# first option
(?:[-+0 #]{0,5})   # optional flags
(?:\d+|\*)?# width
(?:\.(?:\d+|\*))?  # precision
(?:h|l|ll|w|I|I32|I64)?# size
[cCdiouxXeEfgGaAnpsSZ] # type
) |# OR
%%)# literal "%%"
'''

regex = re.compile(fmtstring, re.X)

def parse_format(f):
return tuple((m.span(), m.group()) for m in
regex.finditer(f))

// covered by others but processing that can take hours on some files
@shortcut@
type T;
identifier buf;
expression I, E, L;
expression list args_before, args, args_after;
expression format1, format2;
position p1, p2;
@@

(
{
\( T buf; \| T buf = I; \| \)
snprintf@p1( buf, E, format1, args );
Debug@p2( L, format2, args_before, buf, args_after );
}
|
{
\( T buf; \| T buf = I; \| \)
snprintf@p1( buf, E, format1, args );
Debug@p2( L, format2, args_before, buf, args_after );
}
)

@script:python shortcut_process@
format1 << shortcut.format1;
format2 << shortcut.format2;
args_before << shortcut.args_before;
merged;
@@

pos = len(args_before.elements)
formats = parse_format(format2)

span, format = formats[pos]
merged = format2[:span[0]] + format1.strip('"') + format2[span[1]:]

coccinelle.merged = merged

@shortcut_replace@
position shortcut.p1, shortcut.p2;
identifier 

Re: [Cocci] Matching format strings

2017-07-11 Thread Julia Lawall


On Tue, 11 Jul 2017, Ondřej Kuzník wrote:

> On Mon, Jul 10, 2017 at 07:26:36PM +0200, Julia Lawall wrote:
> > Try running spatch --parse-c on your directory to get a list at the end of
> > the top 10 things it doesn't like.
>
> One of the system dependent macros SLAP_EVENT_DECL is listed in each of
> the 10 things. The macro is used in
> https://github.com/openldap/openldap/blob/master/servers/slapd/daemon.c#L2343
> to declare some variables used in the corresponding event loop.
>
> The macro definition looks like this:
>
> # define SLAP_EVENT_DECL  struct pollfd *revents
>
> or:
>
> # define SLAP_EVENT_DECL  fd_set readfds, writefds; char *rflags
>
> Ignoring this macro is OK I guess, if there is a way to do that with
> coccinelle?

Just

#define SLAP_EVENT_DECL

in the .h file. But there will be stray ;s then.  So you might want to put
the whole first definition.

>
> Also, moving onto the other patch, I get similar inconsistencies with
> ./servers/slapd/back-sql/add.c, --parse-c doesn't list any problematic
> statements but the output logs a few parse errors that don't make much
> sense to me.

Could you send the complete semantic patch that you are currently
considering?  Then I can take a look.

julia

> Thanks,
> Ondrej
>
> > On Mon, 10 Jul 2017, Ondřej Kuzník wrote:
> >> On Mon, Jul 10, 2017 at 06:18:14PM +0200, Julia Lawall wrote:
> >>> On Mon, 10 Jul 2017, Ondřej Kuzník wrote:
>  Thanks, good thing is make test now seems to pass for the first round of
>  patches.
> 
>  Actually, I'm now having a similar issue with ./servers/slapd/daemon.c,
>  spatch --parse-c will complain about the SLAP_EVENT_DECL macro and I can
>  see other issues with LDAP_LIST/LDAP_STAILQ/... macros in the output.
> 
>  This patch is the one that's affected (among others), it will make some
>  changes, applying them and then running it again will make more until
>  eventually managing to pick up most or all of the ones that I'd expect:
> 
>  @@
>  format list[2] fmt;
>  expression list[2] args;
>  expression E;
>  @@
> 
>  Debug( E, "%@fmt@", args
>  -, 0
>   );
> 
>  @@
>  format list[1] fmt;
>  expression list[1] args;
>  expression E;
>  @@
> 
>  Debug( E, "%@fmt@", args
>  -, 0, 0
>   );
> 
>  @@
>  expression E, S;
>  @@
> 
>  Debug( E, S
>  -, 0, 0, 0
>   );
> 
>  Is there a way to work around that somehow?
> >>>
> >>> I'm not sure to understand the relation between the macro problem and the
> >>> need to iterate.
> >>
> >> The above patch will make some changes but not all (even though the
> >> patch is idempotent) but iteration seems to help. It seems that it's
> >> because spatch does not like the file (or rather some of the macros
> >> there).
> >>
> >> --
> >> Ondřej Kuzník
> >> Senior Software Engineer
> >> Symas Corporation   http://www.symas.com
> >> Packaged, certified, and supported LDAP solutions powered by OpenLDAP
> >>
>
>
> --
> Ondřej Kuzník
> Senior Software Engineer
> Symas Corporation   http://www.symas.com
> Packaged, certified, and supported LDAP solutions powered by OpenLDAP
>___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci


Re: [Cocci] Matching format strings

2017-07-11 Thread Ondřej Kuzník
On Mon, Jul 10, 2017 at 07:26:36PM +0200, Julia Lawall wrote:
> Try running spatch --parse-c on your directory to get a list at the end of
> the top 10 things it doesn't like.

One of the system dependent macros SLAP_EVENT_DECL is listed in each of
the 10 things. The macro is used in
https://github.com/openldap/openldap/blob/master/servers/slapd/daemon.c#L2343
to declare some variables used in the corresponding event loop.

The macro definition looks like this:

# define SLAP_EVENT_DECLstruct pollfd *revents

or:

# define SLAP_EVENT_DECLfd_set readfds, writefds; char *rflags

Ignoring this macro is OK I guess, if there is a way to do that with
coccinelle?

Also, moving onto the other patch, I get similar inconsistencies with
./servers/slapd/back-sql/add.c, --parse-c doesn't list any problematic
statements but the output logs a few parse errors that don't make much
sense to me.

Thanks,
Ondrej

> On Mon, 10 Jul 2017, Ondřej Kuzník wrote:
>> On Mon, Jul 10, 2017 at 06:18:14PM +0200, Julia Lawall wrote:
>>> On Mon, 10 Jul 2017, Ondřej Kuzník wrote:
 Thanks, good thing is make test now seems to pass for the first round of
 patches.

 Actually, I'm now having a similar issue with ./servers/slapd/daemon.c,
 spatch --parse-c will complain about the SLAP_EVENT_DECL macro and I can
 see other issues with LDAP_LIST/LDAP_STAILQ/... macros in the output.

 This patch is the one that's affected (among others), it will make some
 changes, applying them and then running it again will make more until
 eventually managing to pick up most or all of the ones that I'd expect:

 @@
 format list[2] fmt;
 expression list[2] args;
 expression E;
 @@

 Debug( E, "%@fmt@", args
 -, 0
  );

 @@
 format list[1] fmt;
 expression list[1] args;
 expression E;
 @@

 Debug( E, "%@fmt@", args
 -, 0, 0
  );

 @@
 expression E, S;
 @@

 Debug( E, S
 -, 0, 0, 0
  );

 Is there a way to work around that somehow?
>>>
>>> I'm not sure to understand the relation between the macro problem and the
>>> need to iterate.
>>
>> The above patch will make some changes but not all (even though the
>> patch is idempotent) but iteration seems to help. It seems that it's
>> because spatch does not like the file (or rather some of the macros
>> there).
>>
>> --
>> Ondřej Kuzník
>> Senior Software Engineer
>> Symas Corporation   http://www.symas.com
>> Packaged, certified, and supported LDAP solutions powered by OpenLDAP
>>


-- 
Ondřej Kuzník
Senior Software Engineer
Symas Corporation   http://www.symas.com
Packaged, certified, and supported LDAP solutions powered by OpenLDAP
___
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci


Re: [Cocci] Replacing suffix rules in make scripts?

2017-07-11 Thread SF Markus Elfring
> So I get the impression that an other build approach will be safer there.
> How do you think about to add a dependency specification so that interface
> descriptions will be always compiled before the corresponding OCaml code?

I showed change possibilities which can improve the software situation
to some degree.

* Improve several dependency specifications in make rules
  https://github.com/coccinelle/coccinelle/pull/108

* Fix specifications for build dependencies between shared components
  https://github.com/coccinelle/coccinelle/issues/110

* Support references for the top-level source file directory in build scripts
  https://github.com/coccinelle/coccinelle/issues/104


Which ideas would you like to integrate from the available development 
approaches?

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