Re: [Cocci] Matching format strings
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
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
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
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?
> 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