* [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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.