diff options
author | Colin Walters <walters@verbum.org> | 2017-03-23 13:06:07 -0400 |
---|---|---|
committer | Atomic Bot <atomic-devel@projectatomic.io> | 2017-03-30 19:19:54 +0000 |
commit | 305db981d47e4aa417dd90164b1995e008d25757 (patch) | |
tree | 6a391e2627f8969ced1ecc5ff2b2d60c36c03b5a | |
parent | ee626c2654b877f5ae6771933330c381efa7e802 (diff) | |
download | ostree-305db981d47e4aa417dd90164b1995e008d25757.tar.gz |
Add Coccinelle usage: one for blacklisting, one for patch collection
This is inspired by the [Coccinelle](http://coccinelle.lip6.fr/) usage
in systemd. I also took it a bit further and added infrastructure
to have spatches which should never apply. This acts as a blacklist.
The reason to do the latter is that coccinelle is *way* more powerful than the
regular expresssions we have in `make syntax-check`.
I started with blacklisting `g_error_free()` directly. The reason that's bad is
it leaves a dangling pointer.
Closes: #754
Approved by: jlebon
-rw-r--r-- | .redhat-ci.Dockerfile | 1 | ||||
-rw-r--r-- | .redhat-ci.yml | 1 | ||||
-rw-r--r-- | Makefile-tests.am | 2 | ||||
-rw-r--r-- | coccinelle/README.md | 6 | ||||
-rw-r--r-- | coccinelle/newstyle.cocci | 22 | ||||
-rw-r--r-- | src/libostree/ostree-repo-pull.c | 41 | ||||
-rw-r--r-- | src/ostree/main.c | 3 | ||||
-rw-r--r-- | src/ostree/ot-main.c | 5 | ||||
-rwxr-xr-x | tests/coccinelle.sh | 29 | ||||
-rw-r--r-- | tests/coccinelle/README.md | 2 | ||||
-rw-r--r-- | tests/coccinelle/raw-free.cocci | 5 | ||||
-rw-r--r-- | tests/test-rollsum-cli.c | 5 |
12 files changed, 96 insertions, 26 deletions
diff --git a/.redhat-ci.Dockerfile b/.redhat-ci.Dockerfile index d5a2e255..86d26dfa 100644 --- a/.redhat-ci.Dockerfile +++ b/.redhat-ci.Dockerfile @@ -8,6 +8,7 @@ RUN dnf install -y \ fuse \ gjs \ parallel \ + coccinelle \ clang \ libubsan \ libasan \ diff --git a/.redhat-ci.yml b/.redhat-ci.yml index cfff0fa0..2fce02b8 100644 --- a/.redhat-ci.yml +++ b/.redhat-ci.yml @@ -11,6 +11,7 @@ container: packages: - libasan + - coccinelle env: CFLAGS: '-fsanitize=undefined -fsanitize-undefined-trap-on-error -fsanitize=address -O2 -Wp,-D_FORTIFY_SOURCE=2' diff --git a/Makefile-tests.am b/Makefile-tests.am index bba8f008..8389331d 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -28,6 +28,7 @@ EXTRA_DIST += \ # include the builddir in $PATH so we find our just-built ostree # binary. TESTS_ENVIRONMENT += OT_TESTS_DEBUG=1 \ + OSTREE_UNINSTALLED_SRCDIR=$(abs_top_srcdir) \ OSTREE_UNINSTALLED=$(abs_top_builddir) \ G_DEBUG=fatal-warnings \ GI_TYPELIB_PATH=$$(cd $(top_builddir) && pwd)$${GI_TYPELIB_PATH:+:$$GI_TYPELIB_PATH} \ @@ -99,6 +100,7 @@ dist_test_scripts = \ tests/test-switchroot.sh \ tests/test-pull-contenturl.sh \ tests/test-pull-mirrorlist.sh \ + tests/coccinelle.sh \ $(NULL) if BUILDOPT_FUSE diff --git a/coccinelle/README.md b/coccinelle/README.md new file mode 100644 index 00000000..60909b1f --- /dev/null +++ b/coccinelle/README.md @@ -0,0 +1,6 @@ +This is a directory of semantic patches +to apply with coccinelle, like the collection in systemd: +https://github.com/systemd/systemd/tree/29f32655842a0712e8db482bcefc4da8908460c8/coccinelle + +See also the tests/coccinelle directory which +has spatches which detect errors. diff --git a/coccinelle/newstyle.cocci b/coccinelle/newstyle.cocci new file mode 100644 index 00000000..7df248c3 --- /dev/null +++ b/coccinelle/newstyle.cocci @@ -0,0 +1,22 @@ +@@ +expression p; +@@ +- glnx_set_error_from_errno (p); +- goto out; ++ return glnx_throw_errno (p); +@@ +expression p; +@@ +- if (!p) +- goto out; ++ if (!p) ++ return FALSE; +@@ +expression p; +@@ +- gboolean ret = FALSE; +... +- ret = TRUE; +- out: +- return ret; ++ return TRUE; diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index b7be8a95..a7a3a5b0 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -279,18 +279,21 @@ pull_termination_condition (OtPullData *pull_data) static void check_outstanding_requests_handle_error (OtPullData *pull_data, - GError *error) + GError **errorp) { + g_assert (errorp); + + GError *error = *errorp; if (error) { if (!pull_data->caught_error) { pull_data->caught_error = TRUE; - g_propagate_error (pull_data->async_error, error); + g_propagate_error (pull_data->async_error, g_steal_pointer (errorp)); } else { - g_error_free (error); + g_clear_error (errorp); } } else @@ -382,7 +385,7 @@ idle_worker (gpointer user_data) { OtPullData *pull_data = user_data; ScanObjectQueueData *scan_data; - GError *error = NULL; + g_autoptr(GError) error = NULL; scan_data = g_queue_pop_head (&pull_data->scan_object_queue); if (!scan_data) @@ -398,7 +401,7 @@ idle_worker (gpointer user_data) scan_data->recursion_depth, pull_data->cancellable, &error); - check_outstanding_requests_handle_error (pull_data, error); + check_outstanding_requests_handle_error (pull_data, &error); g_free (scan_data->path); g_free (scan_data); @@ -760,7 +763,7 @@ content_fetch_on_write_complete (GObject *object, { FetchObjectData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; GError **error = &local_error; OstreeObjectType objtype; const char *expected_checksum; @@ -794,7 +797,7 @@ content_fetch_on_write_complete (GObject *object, pull_data->n_fetched_deltapart_fallbacks++; out: pull_data->n_outstanding_content_write_requests--; - check_outstanding_requests_handle_error (pull_data, local_error); + check_outstanding_requests_handle_error (pull_data, &local_error); fetch_object_data_free (fetch_data); } @@ -806,7 +809,7 @@ content_fetch_on_complete (GObject *object, OstreeFetcher *fetcher = (OstreeFetcher *)object; FetchObjectData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; GError **error = &local_error; GCancellable *cancellable = NULL; guint64 length; @@ -881,7 +884,7 @@ content_fetch_on_complete (GObject *object, out: pull_data->n_outstanding_content_fetches--; - check_outstanding_requests_handle_error (pull_data, local_error); + check_outstanding_requests_handle_error (pull_data, &local_error); if (free_fetch_data) fetch_object_data_free (fetch_data); } @@ -893,7 +896,7 @@ on_metadata_written (GObject *object, { FetchObjectData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; GError **error = &local_error; const char *expected_checksum; OstreeObjectType objtype; @@ -927,7 +930,7 @@ on_metadata_written (GObject *object, pull_data->n_outstanding_metadata_write_requests--; fetch_object_data_free (fetch_data); - check_outstanding_requests_handle_error (pull_data, local_error); + check_outstanding_requests_handle_error (pull_data, &local_error); } static void @@ -943,7 +946,7 @@ meta_fetch_on_complete (GObject *object, const char *checksum; g_autofree char *checksum_obj = NULL; OstreeObjectType objtype; - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; GError **error = &local_error; glnx_fd_close int fd = -1; gboolean free_fetch_data = TRUE; @@ -1038,7 +1041,7 @@ meta_fetch_on_complete (GObject *object, g_assert (pull_data->n_outstanding_metadata_fetches > 0); pull_data->n_outstanding_metadata_fetches--; pull_data->n_fetched_metadata++; - check_outstanding_requests_handle_error (pull_data, local_error); + check_outstanding_requests_handle_error (pull_data, &local_error); if (free_fetch_data) fetch_object_data_free (fetch_data); } @@ -1061,7 +1064,7 @@ on_static_delta_written (GObject *object, { FetchStaticDeltaData *fetch_data = user_data; OtPullData *pull_data = fetch_data->pull_data; - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; GError **error = &local_error; g_debug ("execute static delta part %s complete", fetch_data->expected_checksum); @@ -1072,7 +1075,7 @@ on_static_delta_written (GObject *object, out: g_assert (pull_data->n_outstanding_deltapart_write_requests > 0); pull_data->n_outstanding_deltapart_write_requests--; - check_outstanding_requests_handle_error (pull_data, local_error); + check_outstanding_requests_handle_error (pull_data, &local_error); /* Always free state */ fetch_static_delta_data_free (fetch_data); } @@ -1088,7 +1091,7 @@ static_deltapart_fetch_on_complete (GObject *object, g_autofree char *temp_path = NULL; g_autoptr(GInputStream) in = NULL; g_autoptr(GVariant) part = NULL; - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; GError **error = &local_error; glnx_fd_close int fd = -1; gboolean free_fetch_data = TRUE; @@ -1132,7 +1135,7 @@ static_deltapart_fetch_on_complete (GObject *object, g_assert (pull_data->n_outstanding_deltapart_fetches > 0); pull_data->n_outstanding_deltapart_fetches--; pull_data->n_fetched_deltaparts++; - check_outstanding_requests_handle_error (pull_data, local_error); + check_outstanding_requests_handle_error (pull_data, &local_error); if (free_fetch_data) fetch_static_delta_data_free (fetch_data); } @@ -1968,7 +1971,7 @@ on_superblock_fetched (GObject *src, { FetchDeltaSuperData *fdata = data; OtPullData *pull_data = fdata->pull_data; - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; GError **error = &local_error; g_autoptr(GBytes) delta_superblock_data = NULL; const char *from_revision = fdata->from_revision; @@ -2045,7 +2048,7 @@ on_superblock_fetched (GObject *src, g_assert (pull_data->n_outstanding_metadata_fetches > 0); pull_data->n_outstanding_metadata_fetches--; pull_data->n_fetched_metadata++; - check_outstanding_requests_handle_error (pull_data, local_error); + check_outstanding_requests_handle_error (pull_data, &local_error); } static gboolean diff --git a/src/ostree/main.c b/src/ostree/main.c index c6dee4ee..5a2ed661 100644 --- a/src/ostree/main.c +++ b/src/ostree/main.c @@ -68,7 +68,7 @@ int main (int argc, char **argv) { - GError *error = NULL; + g_autoptr(GError) error = NULL; int ret; setlocale (LC_ALL, ""); @@ -88,7 +88,6 @@ main (int argc, suffix = "\x1b[22m\x1b[0m"; /* bold off, color reset */ } g_printerr ("%serror: %s%s\n", prefix, suffix, error->message); - g_error_free (error); } return ret; diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index 3484b18e..7eb65602 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -259,7 +259,7 @@ ostree_option_context_parse (GOptionContext *context, if (opt_repo == NULL && !(flags & OSTREE_BUILTIN_FLAG_NO_REPO)) { - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; repo = ostree_repo_new_default (); if (!ostree_repo_open (repo, cancellable, &local_error)) @@ -270,14 +270,13 @@ ostree_option_context_parse (GOptionContext *context, g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Command requires a --repo argument"); - g_error_free (local_error); help = g_option_context_get_help (context, FALSE, NULL); g_printerr ("%s", help); } else { - g_propagate_error (error, local_error); + g_propagate_error (error, g_steal_pointer (&local_error)); } goto out; } diff --git a/tests/coccinelle.sh b/tests/coccinelle.sh new file mode 100755 index 00000000..eb22662d --- /dev/null +++ b/tests/coccinelle.sh @@ -0,0 +1,29 @@ +#!/usr/bin/env bash + +# Run the .cocci files in the tests directory; these act +# as a blacklist. + +set -euo pipefail + +. $(dirname $0)/libtest.sh + +if ! spatch --version 2>/dev/null; then + skip "no spatch; get it from http://coccinelle.lip6.fr/" +fi + +if test -z "${OSTREE_UNINSTALLED_SRCDIR:-}"; then + skip "running installed?" +fi + +coccitests=$(ls $(dirname $0)/coccinelle/*.cocci) +echo "1.."$(echo ${coccitests} | wc -l) + +for cocci in $(dirname $0)/coccinelle/*.cocci; do + echo "Running: ${cocci}" + spatch --very-quiet --dir ${OSTREE_UNINSTALLED_SRCDIR} ${cocci} > cocci.out + if test -s cocci.out; then + sed -e 's/^/# /' < cocci.out >&2 + fatal "Failed semantic patch: ${cocci}" + fi + echo ok ${cocci} +done diff --git a/tests/coccinelle/README.md b/tests/coccinelle/README.md new file mode 100644 index 00000000..b81dada3 --- /dev/null +++ b/tests/coccinelle/README.md @@ -0,0 +1,2 @@ +Add patches here which should never match in the code; i.e. the suggested +replacement may be junk. diff --git a/tests/coccinelle/raw-free.cocci b/tests/coccinelle/raw-free.cocci new file mode 100644 index 00000000..9b63b7a5 --- /dev/null +++ b/tests/coccinelle/raw-free.cocci @@ -0,0 +1,5 @@ +@@ +expression p; +@@ +- g_error_free (p); ++ g_clear_error (&p); diff --git a/tests/test-rollsum-cli.c b/tests/test-rollsum-cli.c index a00e4b73..256c79fe 100644 --- a/tests/test-rollsum-cli.c +++ b/tests/test-rollsum-cli.c @@ -22,10 +22,12 @@ #include "ostree-rollsum.h" +#include "libglnx.h" + int main (int argc, char **argv) { - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; GError **error = &local_error; GBytes *from_bytes = NULL; GBytes *to_bytes = NULL; @@ -64,7 +66,6 @@ main (int argc, char **argv) if (local_error) { g_printerr ("%s\n", local_error->message); - g_error_free (local_error); return 1; } return 0; |