* [Cocci] Checking signal handler implementations with SmPL
@ 2015-07-15 7:09 SF Markus Elfring
2015-07-15 14:27 ` Julia Lawall
2015-07-16 5:41 ` [Cocci] Deletion of blank lines? SF Markus Elfring
0 siblings, 2 replies; 6+ messages in thread
From: SF Markus Elfring @ 2015-07-15 7:09 UTC (permalink / raw)
To: cocci
Hello,
I am looking for a specific programming mistake with the help
of the semantic patch language once more.
1. A SmPL approach like the following seems to work to some degree.
@find_unsafe@
identifier receiver =~ "^(?x)
(?:
(?:alarm|parent|sig(?:chld|hup|term))_handler
|
handle_sigterm
|
(?:CancelJ|cancel_j)ob
)$";
@@
void receiver(...)
{
...
(
- exit(...);
|
- fprintf(...);
)
...
}
elfring at Sonne:~/Projekte/CUPS> spatch.opt -sp-file async-unsafe1.cocci -dir lokal > async-unsafe1.diff && cat async-unsafe1.diff
init_defs_builtins: /usr/local/lib/coccinelle/standard.h
?
Skipping:lokal/vcnet/regex/regerror.c
diff -u -p a/backend/ipp.c b/backend/ipp.c
--- a/backend/ipp.c
+++ b/backend/ipp.c
@@ -3271,8 +3271,6 @@ sigterm_handler(int sig) /* I - Signal
if (tmpfilename[0])
unlink(tmpfilename);
-
- exit(1);
2. I have tried another SmPL script variant out.
@show_unsafe@
identifier receiver =~ "^(?x)
(?:
(?:alarm|parent|sig(?:chld|hup|term))_handler
|
handle_sigterm
|
(?:CancelJ|cancel_j)ob
)$";
@@
void receiver(...)
{
...
(
* exit
|
* fprintf
)
(...);
...
}
elfring at Sonne:~/Projekte/CUPS> spatch.opt -sp-file async-unsafe2.cocci -dir lokal > async-unsafe2.diff && cat async-unsafe2.diff
init_defs_builtins: /usr/local/lib/coccinelle/standard.h
?
diff -u -p lokal/backend/dnssd.c /tmp/nothing/backend/dnssd.c
--- lokal/backend/dnssd.c
+++ /tmp/nothing/backend/dnssd.c
@@ -1289,7 +1289,6 @@ sigterm_handler(int sig) /* I - Signal
(void)sig;
if (job_canceled)
- exit(CUPS_BACKEND_OK);
else
job_canceled = 1;
}
diff -u -p lokal/backend/usb-darwin.c /tmp/nothing/backend/usb-darwin.c
--- lokal/backend/usb-darwin.c
+++ /tmp/nothing/backend/usb-darwin.c
@@ -2262,9 +2262,7 @@ sigterm_handler(int sig) /* I - Signal
while (waitpid(child_pid, &status, 0) < 0 && errno == EINTR);
if (WIFEXITED(status))
- exit(WEXITSTATUS(status));
else if (status == SIGTERM || status == SIGKILL)
- exit(0);
else
{
fprintf(stderr, "DEBUG: Child crashed on signal %d\n", status);
diff -u -p lokal/backend/ipp.c /tmp/nothing/backend/ipp.c
--- lokal/backend/ipp.c
+++ /tmp/nothing/backend/ipp.c
@@ -3272,7 +3272,6 @@ sigterm_handler(int sig) /* I - Signal
if (tmpfilename[0])
unlink(tmpfilename);
- exit(1);
}
Why did the first approach find less?
Should the call of the function "fprintf" in the source file "usb-darwin.c"
also be marked for further considerations here?
Are there any more update candidates to consider?
3. If I add the parameter "-jobs 4" to the shown commands for a parallel
source code analysis, I wonder about a message like the following.
[Pid 5399]: Error creating async-unsafe2 : File exists; proceeding without stdout/stderr redirection
Should it be avoided usually?
Regards,
Markus
^ permalink raw reply [flat|nested] 6+ messages in thread* [Cocci] Checking signal handler implementations with SmPL 2015-07-15 7:09 [Cocci] Checking signal handler implementations with SmPL SF Markus Elfring @ 2015-07-15 14:27 ` Julia Lawall 2015-07-15 20:38 ` SF Markus Elfring 2015-07-16 5:41 ` [Cocci] Deletion of blank lines? SF Markus Elfring 1 sibling, 1 reply; 6+ messages in thread From: Julia Lawall @ 2015-07-15 14:27 UTC (permalink / raw) To: cocci On Wed, 15 Jul 2015, SF Markus Elfring wrote: > Hello, > > I am looking for a specific programming mistake with the help > of the semantic patch language once more. > > > 1. A SmPL approach like the following seems to work to some degree. > > @find_unsafe@ > identifier receiver =~ "^(?x) > (?: > (?:alarm|parent|sig(?:chld|hup|term))_handler > | > handle_sigterm > | > (?:CancelJ|cancel_j)ob > )$"; > @@ > void receiver(...) > { > ... > ( > - exit(...); > | > - fprintf(...); > ) > ... > } When you make a transformation, you get a forall semantics - every control flow path in the function must respect your pattern. When you search for something with * you get an exists semantics - there must exist a path that has the patern you specify. If you don't like these defaults, you can change them, by putting exists or forall in the initial @@, or by putting when exists or when forall on the ... Furthermore, ... matches the shortest path between two points. That is, in your rule, you will only match the case where a control-flow patch contains exactly one call to either exit or fprintf. If you want to allow more occurrences, you can use <... ...> with searches for 0 or more or something, or <+... ...+> which searches for one or more. I believe the parallelism error has been fixed. I'm not sure if it was in the release, though. If your semantic patch is called foo.cocci, check that you don't have a file/directory called foo in the current directory. julia > > elfring at Sonne:~/Projekte/CUPS> spatch.opt -sp-file async-unsafe1.cocci -dir lokal > async-unsafe1.diff && cat async-unsafe1.diff > init_defs_builtins: /usr/local/lib/coccinelle/standard.h > ? > Skipping:lokal/vcnet/regex/regerror.c > diff -u -p a/backend/ipp.c b/backend/ipp.c > --- a/backend/ipp.c > +++ b/backend/ipp.c > @@ -3271,8 +3271,6 @@ sigterm_handler(int sig) /* I - Signal > > if (tmpfilename[0]) > unlink(tmpfilename); > - > - exit(1); > > > > 2. I have tried another SmPL script variant out. > > @show_unsafe@ > identifier receiver =~ "^(?x) > (?: > (?:alarm|parent|sig(?:chld|hup|term))_handler > | > handle_sigterm > | > (?:CancelJ|cancel_j)ob > )$"; > @@ > void receiver(...) > { > ... > ( > * exit > |f > * fprintf > ) > (...); > ... > } > > elfring at Sonne:~/Projekte/CUPS> spatch.opt -sp-file async-unsafe2.cocci -dir lokal > async-unsafe2.diff && cat async-unsafe2.diff > init_defs_builtins: /usr/local/lib/coccinelle/standard.h > ? > diff -u -p lokal/backend/dnssd.c /tmp/nothing/backend/dnssd.c > --- lokal/backend/dnssd.c > +++ /tmp/nothing/backend/dnssd.c > @@ -1289,7 +1289,6 @@ sigterm_handler(int sig) /* I - Signal > (void)sig; > > if (job_canceled) > - exit(CUPS_BACKEND_OK); > else > job_canceled = 1; > } > diff -u -p lokal/backend/usb-darwin.c /tmp/nothing/backend/usb-darwin.c > --- lokal/backend/usb-darwin.c > +++ /tmp/nothing/backend/usb-darwin.c > @@ -2262,9 +2262,7 @@ sigterm_handler(int sig) /* I - Signal > while (waitpid(child_pid, &status, 0) < 0 && errno == EINTR); > > if (WIFEXITED(status)) > - exit(WEXITSTATUS(status)); > else if (status == SIGTERM || status == SIGKILL) > - exit(0); > else > { > fprintf(stderr, "DEBUG: Child crashed on signal %d\n", status); > diff -u -p lokal/backend/ipp.c /tmp/nothing/backend/ipp.c > --- lokal/backend/ipp.c > +++ /tmp/nothing/backend/ipp.c > @@ -3272,7 +3272,6 @@ sigterm_handler(int sig) /* I - Signal > if (tmpfilename[0]) > unlink(tmpfilename); > > - exit(1); > } > > > Why did the first approach find less? > > > Should the call of the function "fprintf" in the source file "usb-darwin.c" > also be marked for further considerations here? > Are there any more update candidates to consider? > > > > 3. If I add the parameter "-jobs 4" to the shown commands for a parallel > source code analysis, I wonder about a message like the following. > > [Pid 5399]: Error creating async-unsafe2 : File exists; proceeding without stdout/stderr redirection > > Should it be avoided usually? > > Regards, > Markus > _______________________________________________ > Cocci mailing list > Cocci at systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Cocci] Checking signal handler implementations with SmPL 2015-07-15 14:27 ` Julia Lawall @ 2015-07-15 20:38 ` SF Markus Elfring 0 siblings, 0 replies; 6+ messages in thread From: SF Markus Elfring @ 2015-07-15 20:38 UTC (permalink / raw) To: cocci > Furthermore, ... matches the shortest path between two points. It seems that I am still not so used to all combinations of the SmPL ellipsis. > That is, in your rule, you will only match the case where a control-flow patch > contains exactly one call to either exit or fprintf. If you want to allow > more occurrences, you can use <... ...> with searches for 0 or more or > something, or <+... ...+> which searches for one or more. I guess that the last option is more appropriate for my source code search pattern. Thanks for your explanation. Regards, Markus ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Cocci] Deletion of blank lines? 2015-07-15 7:09 [Cocci] Checking signal handler implementations with SmPL SF Markus Elfring 2015-07-15 14:27 ` Julia Lawall @ 2015-07-16 5:41 ` SF Markus Elfring 2015-07-17 9:50 ` Julia Lawall 1 sibling, 1 reply; 6+ messages in thread From: SF Markus Elfring @ 2015-07-16 5:41 UTC (permalink / raw) To: cocci > void receiver(...) > { > ... > ( > - exit(...); > | > - fprintf(...); > ) > ... > } > ? > diff -u -p a/backend/ipp.c b/backend/ipp.c > --- a/backend/ipp.c > +++ b/backend/ipp.c > @@ -3271,8 +3271,6 @@ sigterm_handler(int sig) /* I - Signal > > if (tmpfilename[0]) > unlink(tmpfilename); > - > - exit(1); I find another detail interesting in this generated patch. The Coccinelle software suggests here to delete also a blank line at a specific source code place. I would usually interpret this SmPL script example in the way that I did not express the removal of blank lines. How often do you care if additional source code reformatting should happen around a specific change (or not)? Regards, Markus ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Cocci] Deletion of blank lines? 2015-07-16 5:41 ` [Cocci] Deletion of blank lines? SF Markus Elfring @ 2015-07-17 9:50 ` Julia Lawall 2015-07-17 11:12 ` SF Markus Elfring 0 siblings, 1 reply; 6+ messages in thread From: Julia Lawall @ 2015-07-17 9:50 UTC (permalink / raw) To: cocci On Thu, 16 Jul 2015, SF Markus Elfring wrote: > > void receiver(...) > > { > > ... > > ( > > - exit(...); > > | > > - fprintf(...); > > ) > > ... > > } > > ? > > diff -u -p a/backend/ipp.c b/backend/ipp.c > > --- a/backend/ipp.c > > +++ b/backend/ipp.c > > @@ -3271,8 +3271,6 @@ sigterm_handler(int sig) /* I - Signal > > > > if (tmpfilename[0]) > > unlink(tmpfilename); > > - > > - exit(1); > > I find another detail interesting in this generated patch. > The Coccinelle software suggests here to delete also a blank line > at a specific source code place. I would usually interpret > this SmPL script example in the way that I did not express the removal > of blank lines. Is there a blank line after the call to exit(1); also? I think that it tries toavoid creeating two blank lines. julia > > How often do you care if additional source code reformatting should > happen around a specific change (or not)? > > Regards, > Markus > _______________________________________________ > Cocci mailing list > Cocci at systeme.lip6.fr > https://systeme.lip6.fr/mailman/listinfo/cocci > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Cocci] Deletion of blank lines? 2015-07-17 9:50 ` Julia Lawall @ 2015-07-17 11:12 ` SF Markus Elfring 0 siblings, 0 replies; 6+ messages in thread From: SF Markus Elfring @ 2015-07-17 11:12 UTC (permalink / raw) To: cocci > Is there a blank line after the call to exit(1); also? No. The text line which follows the shown exit() call should only contain the closing curly bracket for the sigterm_handler() function according to the commit "Load cups into easysw/current." (ef416fc25c4af449e930416117bedb12fc9924ba) from 2006-01-13. > I think that it tries toavoid creeating two blank lines. Should the blank line between the shown calls be usually preserved by default? When would the suggested functionality (like a bit of extra source code clean-up) be really needed? Regards, Markus ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-07-17 11:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-07-15 7:09 [Cocci] Checking signal handler implementations with SmPL SF Markus Elfring 2015-07-15 14:27 ` Julia Lawall 2015-07-15 20:38 ` SF Markus Elfring 2015-07-16 5:41 ` [Cocci] Deletion of blank lines? SF Markus Elfring 2015-07-17 9:50 ` Julia Lawall 2015-07-17 11:12 ` SF Markus Elfring
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox