From 70d140d61c259717fd9616c2e37eaae6b7338ae7 Mon Sep 17 00:00:00 2001 From: "Jasper St. Pierre" Date: Wed, 23 Nov 2016 13:52:39 -0800 Subject: [PATCH 01/55] ostree-repo-traverse: Remove an accidental print statement Closes: #594 Approved by: jlebon --- src/libostree/ostree-repo-traverse.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-traverse.c b/src/libostree/ostree-repo-traverse.c index 5d994763..e620a8ad 100644 --- a/src/libostree/ostree-repo-traverse.c +++ b/src/libostree/ostree-repo-traverse.c @@ -407,7 +407,7 @@ traverse_dirtree (OstreeRepo *repo, if (ignore_missing_dirs && g_error_matches (local_error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { - g_print ("Ignoring not-found dirmeta %s", checksum); + g_debug ("Ignoring not-found dirmeta %s", checksum); ret = TRUE; } else From c9b158e9f2bae49cd16fd1d1cf96813cb1e0440d Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 28 Nov 2016 13:58:19 +0100 Subject: [PATCH 02/55] pull: scan_commit_object() - don't load variant twice ostree_repo_load_commit already loaded the object, no need to load it twice. Closes: #595 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 9e960792..6f32c77b 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1137,10 +1137,6 @@ scan_commit_object (OtPullData *pull_data, is_partial = pull_data->legacy_transaction_resuming || (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) > 0; - if (!ostree_repo_load_variant (pull_data->repo, OSTREE_OBJECT_TYPE_COMMIT, checksum, - &commit, error)) - goto out; - /* PARSE OSTREE_SERIALIZED_COMMIT_VARIANT */ g_variant_get_child (commit, 1, "@ay", &parent_csum); if (g_variant_n_children (parent_csum) > 0) From fc107e5bb40c80669fe150caf90a23e83447a4d4 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 28 Nov 2016 15:31:04 +0100 Subject: [PATCH 03/55] ostree-repo-traverse: Don't leak floating GVariant ostree_object_name_serialize returns a floating ref, so we need to sink it before putting in the hashtable. Closes: #595 Approved by: cgwalters --- src/libostree/ostree-repo-traverse.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-traverse.c b/src/libostree/ostree-repo-traverse.c index e620a8ad..46f83240 100644 --- a/src/libostree/ostree-repo-traverse.c +++ b/src/libostree/ostree-repo-traverse.c @@ -348,7 +348,7 @@ traverse_iter (OstreeRepo *repo, ostree_repo_commit_traverse_iter_get_file (iter, &name, &checksum); g_debug ("Found file object %s", checksum); - key = ostree_object_name_serialize (checksum, OSTREE_OBJECT_TYPE_FILE); + key = g_variant_ref_sink (ostree_object_name_serialize (checksum, OSTREE_OBJECT_TYPE_FILE)); g_hash_table_replace (inout_reachable, key, key); key = NULL; } @@ -363,11 +363,11 @@ traverse_iter (OstreeRepo *repo, g_debug ("Found dirtree object %s", content_checksum); g_debug ("Found dirmeta object %s", meta_checksum); - key = ostree_object_name_serialize (meta_checksum, OSTREE_OBJECT_TYPE_DIR_META); + key = g_variant_ref_sink (ostree_object_name_serialize (meta_checksum, OSTREE_OBJECT_TYPE_DIR_META)); g_hash_table_replace (inout_reachable, key, key); key = NULL; - key = ostree_object_name_serialize (content_checksum, OSTREE_OBJECT_TYPE_DIR_TREE); + key = g_variant_ref_sink (ostree_object_name_serialize (content_checksum, OSTREE_OBJECT_TYPE_DIR_TREE)); if (!g_hash_table_lookup (inout_reachable, key)) { g_hash_table_replace (inout_reachable, key, key); @@ -463,7 +463,7 @@ ostree_repo_traverse_commit_union (OstreeRepo *repo, OstreeRepoCommitState commitstate; gboolean ignore_missing_dirs = FALSE; - key = ostree_object_name_serialize (commit_checksum, OSTREE_OBJECT_TYPE_COMMIT); + key = g_variant_ref_sink (ostree_object_name_serialize (commit_checksum, OSTREE_OBJECT_TYPE_COMMIT)); if (g_hash_table_contains (inout_reachable, key)) break; From 239cb1494fcb99f963e1d5f7939b8b1a63cf9798 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 28 Nov 2016 15:56:50 +0100 Subject: [PATCH 04/55] pull_with_options: Don't leak csum_v Closes: #596 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 6f32c77b..303daec4 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2800,7 +2800,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, for (i = 0; i < n; i++) { const char *delta; - GVariant *csum_v = NULL; + g_autoptr(GVariant) csum_v = NULL; guchar *csum_data = g_malloc (OSTREE_SHA256_DIGEST_LEN); g_autoptr(GVariant) ref = g_variant_get_child_value (deltas, i); From 36f7824cb05f4952ae2edfd7256c8f85e5bd9a42 Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 28 Nov 2016 15:57:11 +0100 Subject: [PATCH 05/55] pull: Don't leak delta superblock variants Closes: #596 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 303daec4..ffa387a9 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1478,8 +1478,8 @@ request_static_delta_superblock_sync (OtPullData *pull_data, } } - ret_delta_superblock = g_variant_new_from_bytes ((GVariantType*)OSTREE_STATIC_DELTA_SUPERBLOCK_FORMAT, - delta_superblock_data, FALSE); + ret_delta_superblock = g_variant_ref_sink (g_variant_new_from_bytes ((GVariantType*)OSTREE_STATIC_DELTA_SUPERBLOCK_FORMAT, + delta_superblock_data, FALSE)); } ret = TRUE; @@ -2951,7 +2951,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_autofree char *from_revision = NULL; const char *ref = key; const char *to_revision = value; - GVariant *delta_superblock = NULL; + g_autoptr(GVariant) delta_superblock = NULL; if (!ostree_repo_resolve_rev (pull_data->repo, ref, TRUE, &from_revision, error)) From d57036f6a2da1adbc581a258d3e128c4b0623baf Mon Sep 17 00:00:00 2001 From: Alexander Larsson Date: Mon, 28 Nov 2016 17:21:59 +0100 Subject: [PATCH 06/55] delta compilation: Fix leak We need to ref-sik the new varian for g_autoptr to work Closes: #597 Approved by: cgwalters --- .../ostree-repo-static-delta-compilation.c | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index 4b0bc507..22c45e6d 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -1532,17 +1532,17 @@ ostree_repo_static_delta_generate (OstreeRepo *self, /* floating */ GVariant *to_csum_v = ostree_checksum_to_bytes_v (to); - delta_descriptor = g_variant_new ("(@a{sv}t@ay@ay@" OSTREE_COMMIT_GVARIANT_STRING "ay" - "a" OSTREE_STATIC_DELTA_META_ENTRY_FORMAT - "@a" OSTREE_STATIC_DELTA_FALLBACK_FORMAT ")", - g_variant_builder_end (&metadata_builder), - GUINT64_TO_BE (g_date_time_to_unix (now)), - from_csum_v, - to_csum_v, - to_commit, - g_variant_builder_new (G_VARIANT_TYPE ("ay")), - part_headers, - fallback_headers); + delta_descriptor = g_variant_ref_sink (g_variant_new ("(@a{sv}t@ay@ay@" OSTREE_COMMIT_GVARIANT_STRING "ay" + "a" OSTREE_STATIC_DELTA_META_ENTRY_FORMAT + "@a" OSTREE_STATIC_DELTA_FALLBACK_FORMAT ")", + g_variant_builder_end (&metadata_builder), + GUINT64_TO_BE (g_date_time_to_unix (now)), + from_csum_v, + to_csum_v, + to_commit, + g_variant_builder_new (G_VARIANT_TYPE ("ay")), + part_headers, + fallback_headers)); g_date_time_unref (now); } From a73fef3eaacdf7fb902f096d60371cdbdc015181 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 19:05:53 +0000 Subject: [PATCH 07/55] build: clean up ostree-remount if building without systemd This is necessary for "make distcheck" on Travis-CI. Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- Makefile-decls.am | 3 +++ Makefile-switchroot.am | 4 ++++ Makefile-tests.am | 2 -- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Makefile-decls.am b/Makefile-decls.am index df635706..e41586d1 100644 --- a/Makefile-decls.am +++ b/Makefile-decls.am @@ -46,6 +46,9 @@ gsettings_SCHEMAS = ostree_bootdir = $(prefix)/lib/ostree ostree_boot_PROGRAMS = +# This initializes some more variables +include $(top_srcdir)/buildutil/glib-tap.mk + # This is a special facility to chain together hooks easily INSTALL_DATA_HOOKS = install-data-hook: $(INSTALL_DATA_HOOKS) diff --git a/Makefile-switchroot.am b/Makefile-switchroot.am index 0b30a965..6fd2f820 100644 --- a/Makefile-switchroot.am +++ b/Makefile-switchroot.am @@ -17,6 +17,10 @@ if BUILDOPT_SYSTEMD ostree_boot_PROGRAMS += ostree-remount +else +# It is built anyway as a side-effect of having the symlink in tests/, +# and if we declare it here, it gets cleaned up properly +check_PROGRAMS += ostree-remount endif ostree_prepare_root_SOURCES = \ diff --git a/Makefile-tests.am b/Makefile-tests.am index 2160cd55..03e50b9b 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -17,8 +17,6 @@ # Free Software Foundation, Inc., 59 Temple Place - Suite 330, # Boston, MA 02111-1307, USA. -include $(top_srcdir)/buildutil/glib-tap.mk - EXTRA_DIST += \ buildutil/tap-driver.sh \ buildutil/tap-test \ From 54655795c9b83b8d942fa114fc18614ed226dcac Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 12:34:06 +0000 Subject: [PATCH 08/55] ci-build: consistently use yes/no for booleans, not yes/empty Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- tests/ci-build.sh | 2 +- tests/ci-install.sh | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/ci-build.sh b/tests/ci-build.sh index 4aaa47af..7b98820d 100755 --- a/tests/ci-build.sh +++ b/tests/ci-build.sh @@ -50,7 +50,7 @@ ${make} install DESTDIR=$(pwd)/DESTDIR ( cd DESTDIR && find . ) -if [ -n "$ci_sudo" ] && [ -n "$ci_test" ]; then +if [ "$ci_sudo" = yes ] && [ "$ci_test" = yes ]; then sudo ${make} install env \ LD_LIBRARY_PATH=/usr/local/lib \ diff --git a/tests/ci-install.sh b/tests/ci-install.sh index 4aff39e9..8111809f 100755 --- a/tests/ci-install.sh +++ b/tests/ci-install.sh @@ -5,7 +5,7 @@ set -x NULL= : "${ci_docker:=}" -: "${ci_in_docker:=}" +: "${ci_in_docker:=no}" : "${ci_suite:=jessie}" if [ $(id -u) = 0 ]; then @@ -66,7 +66,7 @@ case "$ci_distro" in zlib1g-dev \ ${NULL} - if [ -n "$ci_in_docker" ]; then + if [ "$ci_in_docker" = yes ]; then # Add the user that we will use to do the build inside the # Docker container, and let them use sudo adduser --disabled-password user Date: Tue, 29 Nov 2016 13:05:57 +0000 Subject: [PATCH 09/55] ci-install: add ci_distro Otherwise, we'll fail (due to set -u) if this parameter variable isn't passed. Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- tests/ci-install.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/ci-install.sh b/tests/ci-install.sh index 8111809f..326ab35b 100755 --- a/tests/ci-install.sh +++ b/tests/ci-install.sh @@ -4,6 +4,7 @@ set -euo pipefail set -x NULL= +: "${ci_distro:=debian}" : "${ci_docker:=}" : "${ci_in_docker:=no}" : "${ci_suite:=jessie}" From 0473fb852360c3378585861e79fc43897df271db Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 17:21:46 +0000 Subject: [PATCH 10/55] travis-ci: put an explicit copyright/license on the scripts This is deliberately permissive: a lot of it is generic, and I'm using similar scripts in dbus. Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- tests/ci-build.sh | 22 ++++++++++++++++++++++ tests/ci-install.sh | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/tests/ci-build.sh b/tests/ci-build.sh index 7b98820d..eb6bc678 100755 --- a/tests/ci-build.sh +++ b/tests/ci-build.sh @@ -1,5 +1,27 @@ #!/bin/bash +# Copyright © 2015-2016 Collabora Ltd. +# +# Permission is hereby granted, free of charge, to any person +# obtaining a copy of this software and associated documentation files +# (the "Software"), to deal in the Software without restriction, +# including without limitation the rights to use, copy, modify, merge, +# publish, distribute, sublicense, and/or sell copies of the Software, +# and to permit persons to whom the Software is furnished to do so, +# subject to the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS +# BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN +# ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +# CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + set -euo pipefail set -x diff --git a/tests/ci-install.sh b/tests/ci-install.sh index 326ab35b..fb982dff 100755 --- a/tests/ci-install.sh +++ b/tests/ci-install.sh @@ -1,5 +1,27 @@ #!/bin/bash +# Copyright © 2015-2016 Collabora Ltd. +# +# Permission is hereby granted, free of charge, to any person +# obtaining a copy of this software and associated documentation files +# (the "Software"), to deal in the Software without restriction, +# including without limitation the rights to use, copy, modify, merge, +# publish, distribute, sublicense, and/or sell copies of the Software, +# and to permit persons to whom the Software is furnished to do so, +# subject to the following conditions: +# +# The above copyright notice and this permission notice shall be +# included in all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND +# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS +# BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN +# ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN +# CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +# SOFTWARE. + set -euo pipefail set -x From 49b8ccd8cbfe4a09ee09b85b410bfd8f3e5d1b06 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 17:22:24 +0000 Subject: [PATCH 11/55] travis-ci: Use a non-ostree-specific name for the Docker image This reduces the diff when comparing these scripts with similar glue in dbus or elsewhere. Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- tests/ci-build.sh | 2 +- tests/ci-install.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ci-build.sh b/tests/ci-build.sh index eb6bc678..42c2dcff 100755 --- a/tests/ci-build.sh +++ b/tests/ci-build.sh @@ -40,7 +40,7 @@ if [ -n "$ci_docker" ]; then --env=ci_test="${ci_test}" \ --env=ci_test_fatal="${ci_test_fatal}" \ --privileged \ - ostree-ci \ + ci-image \ tests/ci-build.sh fi diff --git a/tests/ci-install.sh b/tests/ci-install.sh index fb982dff..1b52704c 100755 --- a/tests/ci-install.sh +++ b/tests/ci-install.sh @@ -43,7 +43,7 @@ if [ -n "$ci_docker" ]; then -e "s/@ci_docker@/${ci_docker}/" \ -e "s/@ci_suite@/${ci_suite}/" \ < tests/ci-Dockerfile.in > Dockerfile - exec docker build -t ostree-ci . + exec docker build -t ci-image . fi case "$ci_distro" in From db691b552032bfdc526cb58a06f64c07a6ff527e Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 17:23:08 +0000 Subject: [PATCH 12/55] travis-ci: Move helper function to before we start building anything Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- tests/ci-build.sh | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tests/ci-build.sh b/tests/ci-build.sh index 42c2dcff..ae52787e 100755 --- a/tests/ci-build.sh +++ b/tests/ci-build.sh @@ -44,6 +44,12 @@ if [ -n "$ci_docker" ]; then tests/ci-build.sh fi +maybe_fail_tests () { + if [ "$ci_test_fatal" = yes ]; then + exit 1 + fi +} + NOCONFIGURE=1 ./autogen.sh srcdir="$(pwd)" @@ -58,13 +64,6 @@ make="make -j${ci_parallel} V=1 VERBOSE=1" "$@" ${make} - -maybe_fail_tests () { - if [ "$ci_test_fatal" = yes ]; then - exit 1 - fi -} - [ "$ci_test" = no ] || ${make} check || maybe_fail_tests # TODO: if ostree aims to support distcheck, run that too From a1ab6b50f93cb0553bb96c936a5f1d1752ea564d Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 17:23:43 +0000 Subject: [PATCH 13/55] travis-ci: cat the test log after successful test runs This lets us see which tests were skipped. Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- tests/ci-build.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/ci-build.sh b/tests/ci-build.sh index ae52787e..675a9a31 100755 --- a/tests/ci-build.sh +++ b/tests/ci-build.sh @@ -65,6 +65,7 @@ make="make -j${ci_parallel} V=1 VERBOSE=1" ${make} [ "$ci_test" = no ] || ${make} check || maybe_fail_tests +cat test/test-suite.log || : # TODO: if ostree aims to support distcheck, run that too ${make} install DESTDIR=$(pwd)/DESTDIR @@ -78,6 +79,8 @@ if [ "$ci_sudo" = yes ] && [ "$ci_test" = yes ]; then GI_TYPELIB_PATH=/usr/local/lib/girepository-1.0 \ ${make} installcheck || \ maybe_fail_tests + cat test/test-suite.log || : + env \ LD_LIBRARY_PATH=/usr/local/lib \ GI_TYPELIB_PATH=/usr/local/lib/girepository-1.0 \ From 3d6269ebfb318a51b1714d117f31d8a0dca0eed1 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 17:24:08 +0000 Subject: [PATCH 14/55] travis-ci: Run `make distcheck` too ostree is now actively using that mode. Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- tests/ci-build.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/ci-build.sh b/tests/ci-build.sh index 675a9a31..e4ec703b 100755 --- a/tests/ci-build.sh +++ b/tests/ci-build.sh @@ -66,10 +66,10 @@ make="make -j${ci_parallel} V=1 VERBOSE=1" ${make} [ "$ci_test" = no ] || ${make} check || maybe_fail_tests cat test/test-suite.log || : -# TODO: if ostree aims to support distcheck, run that too +[ "$ci_test" = no ] || ${make} distcheck || maybe_fail_tests +cat test/test-suite.log || : ${make} install DESTDIR=$(pwd)/DESTDIR - ( cd DESTDIR && find . ) if [ "$ci_sudo" = yes ] && [ "$ci_test" = yes ]; then From 0261560b35aef5f1b1968311a7fba8ec06fec575 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 17:24:55 +0000 Subject: [PATCH 15/55] travis-ci: Use "slim" Debian image for testing Documentation and similar files are stripped from this image, making it quicker to install. Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 44b78b1e..bf04939c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,7 +4,7 @@ sudo: required env: - ci_distro=ubuntu ci_suite=trusty - - ci_docker=debian:jessie ci_distro=debian ci_suite=jessie + - ci_docker=debian:jessie-slim ci_distro=debian ci_suite=jessie - ci_docker=ubuntu:xenial ci_distro=ubuntu ci_suite=xenial script: From 9cbe23f4b7bcd3b7803571adb696309818636f37 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 28 Nov 2016 17:27:44 +0000 Subject: [PATCH 16/55] travis-ci: Enable stretch (the future Debian 9), replacing unstable My goal in building ostree for Debian unstable was that we would have good coverage of "new code" paths. However, it was removed for #571 as too much of a moving target. Debian testing is less of a moving target, and in particular is always internally consistent (packages are co-installable), which Debian unstable is not guaranteed to be. Debian 'stretch' is the future Debian 9, which should be released next year. Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index bf04939c..67bea019 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,6 +5,7 @@ sudo: required env: - ci_distro=ubuntu ci_suite=trusty - ci_docker=debian:jessie-slim ci_distro=debian ci_suite=jessie + - ci_docker=debian:stretch-slim ci_distro=debian ci_suite=stretch - ci_docker=ubuntu:xenial ci_distro=ubuntu ci_suite=xenial script: From d1c7eba82ca6fb0cb38b68a0c6ed4ebb97f02f69 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Tue, 29 Nov 2016 13:06:14 +0000 Subject: [PATCH 17/55] travis-ci: document parameter variables Signed-off-by: Simon McVittie Closes: #600 Approved by: cgwalters --- tests/ci-build.sh | 20 ++++++++++++++++++++ tests/ci-install.sh | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/tests/ci-build.sh b/tests/ci-build.sh index e4ec703b..4b736a3c 100755 --- a/tests/ci-build.sh +++ b/tests/ci-build.sh @@ -26,10 +26,30 @@ set -euo pipefail set -x NULL= + +# ci_docker: +# If non-empty, this is the name of a Docker image. ci-install.sh will +# fetch it with "docker pull" and use it as a base for a new Docker image +# named "ci-image" in which we will do our testing. +# +# If empty, we test on "bare metal". +# Typical values: ubuntu:xenial, debian:jessie-slim : "${ci_docker:=}" + +# ci_parallel: +# A number of parallel jobs, passed to make -j : "${ci_parallel:=1}" + +# ci_sudo: +# If yes, assume we can get root using sudo; if no, only use current user : "${ci_sudo:=no}" + +# ci_test: +# If yes, run tests; if no, just build : "${ci_test:=yes}" + +# ci_test_fatal: +# If yes, test failures break the build; if no, they are reported but ignored : "${ci_test_fatal:=yes}" if [ -n "$ci_docker" ]; then diff --git a/tests/ci-install.sh b/tests/ci-install.sh index 1b52704c..dbc86c69 100755 --- a/tests/ci-install.sh +++ b/tests/ci-install.sh @@ -26,9 +26,28 @@ set -euo pipefail set -x NULL= + +# ci_distro: +# OS distribution in which we are testing +# Typical values: ubuntu, debian; maybe fedora in future : "${ci_distro:=debian}" + +# ci_docker: +# If non-empty, this is the name of a Docker image. ci-install.sh will +# fetch it with "docker pull" and use it as a base for a new Docker image +# named "ci-image" in which we will do our testing. : "${ci_docker:=}" + +# ci_in_docker: +# Used internally by ci-install.sh. If yes, we are inside the Docker image +# (ci_docker is empty in this case). : "${ci_in_docker:=no}" + +# ci_suite: +# OS suite (release, branch) in which we are testing. +# Typical values for ci_distro=ubuntu: xenial, trusty +# Typical values for ci_distro=debian: sid, jessie +# Typical values for ci_distro=fedora might be 25, rawhide : "${ci_suite:=jessie}" if [ $(id -u) = 0 ]; then From 340376e7b14fbd37f013f73fa119dd0904deb754 Mon Sep 17 00:00:00 2001 From: Mario Sanchez Prada Date: Wed, 30 Nov 2016 15:41:38 +0000 Subject: [PATCH 18/55] man: Mention bare-user in manpages, along with the other modes Closes: #602 Closes: #603 Approved by: cgwalters --- man/ostree-init.xml | 2 +- man/ostree.repo-config.xml | 2 +- man/ostree.repo.xml | 9 ++++++--- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/man/ostree-init.xml b/man/ostree-init.xml index 4518e244..95f0b162 100644 --- a/man/ostree-init.xml +++ b/man/ostree-init.xml @@ -68,7 +68,7 @@ Boston, MA 02111-1307, USA. ="MODE" - Initialize repository in given mode (bare, archive-z2). Default is "bare". + Initialize repository in given mode (bare, bare-user, archive-z2). Default is "bare". diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 876c9dfc..5e0383a6 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -76,7 +76,7 @@ Boston, MA 02111-1307, USA. mode - One of bare or archive-z2. + One of bare, bare-user or archive-z2. diff --git a/man/ostree.repo.xml b/man/ostree.repo.xml index b8ad4052..1187b66a 100644 --- a/man/ostree.repo.xml +++ b/man/ostree.repo.xml @@ -58,11 +58,14 @@ binaries. It records the Unix uid and gid, permissions, as well as extended attributes. - + - A repository can be in one of two modes; + A repository can be in one of three modes; bare, which is designed as a hard - link source for operating system checkouts, and + link source for operating system checkouts, + bare-user, which is like + bare but works on systems that + run as non-root as well as non-root containers, and archive-z2, which is designed for static HTTP servers. From 79cb421ee2927fa3ded0deb7a42878b129779f86 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 21:11:37 -0500 Subject: [PATCH 19/55] [ASAN] delta compilation: More leak fixes Now that I remembered to do `env G_SLICE=always-malloc`, lots more leaks become apparent. Nothing major. Closes: #598 Approved by: jlebon --- .../ostree-repo-static-delta-compilation.c | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/src/libostree/ostree-repo-static-delta-compilation.c b/src/libostree/ostree-repo-static-delta-compilation.c index 22c45e6d..1611d19e 100644 --- a/src/libostree/ostree-repo-static-delta-compilation.c +++ b/src/libostree/ostree-repo-static-delta-compilation.c @@ -132,7 +132,7 @@ xattr_chunk_hash (const void *vp) { const guint8* name; const guint8* value_data; - GVariant *value = NULL; + g_autoptr(GVariant) value = NULL; gsize value_len; g_variant_get_child (v, i, "(^&ay@ay)", @@ -911,9 +911,8 @@ generate_delta_lowlatency (OstreeRepo *repo, ostree_object_name_deserialize (serialized_key, &checksum, &objtype); - g_variant_ref (serialized_key); if (OSTREE_OBJECT_TYPE_IS_META (objtype)) - g_hash_table_add (new_reachable_metadata, serialized_key); + g_hash_table_add (new_reachable_metadata, g_variant_ref (serialized_key)); else { g_autoptr(GFileInfo) finfo = NULL; @@ -955,8 +954,9 @@ generate_delta_lowlatency (OstreeRepo *repo, } /* We already ship the to commit in the superblock, don't ship it twice */ - g_hash_table_remove (new_reachable_metadata, - ostree_object_name_serialize (to, OSTREE_OBJECT_TYPE_COMMIT)); + { g_autoptr(GVariant) commit = ostree_object_name_serialize (to, OSTREE_OBJECT_TYPE_COMMIT); + g_hash_table_remove (new_reachable_metadata, commit); + } rollsum_optimized_content_objects = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, @@ -1359,8 +1359,8 @@ ostree_repo_static_delta_generate (OstreeRepo *self, for (i = 0; i < builder.parts->len; i++) { OstreeStaticDeltaPartBuilder *part_builder = builder.parts->pdata[i]; - GBytes *payload_b; - GBytes *operations_b; + g_autoptr(GBytes) payload_b; + g_autoptr(GBytes) operations_b; g_autofree guchar *part_checksum = NULL; g_autoptr(GBytes) objtype_checksum_array = NULL; g_autoptr(GBytes) checksum_bytes = NULL; @@ -1415,10 +1415,11 @@ ostree_repo_static_delta_generate (OstreeRepo *self, } /* FIXME - avoid duplicating memory here */ - delta_part = g_variant_new ("(y@ay)", - compression_type_char, - ot_gvariant_new_ay_bytes (g_memory_output_stream_steal_as_bytes (part_payload_out))); - g_variant_ref_sink (delta_part); + { g_autoptr(GBytes) payload = g_memory_output_stream_steal_as_bytes (part_payload_out); + delta_part = g_variant_ref_sink (g_variant_new ("(y@ay)", + compression_type_char, + ot_gvariant_new_ay_bytes (payload))); + } if (inline_parts) { @@ -1532,7 +1533,7 @@ ostree_repo_static_delta_generate (OstreeRepo *self, /* floating */ GVariant *to_csum_v = ostree_checksum_to_bytes_v (to); - delta_descriptor = g_variant_ref_sink (g_variant_new ("(@a{sv}t@ay@ay@" OSTREE_COMMIT_GVARIANT_STRING "ay" + delta_descriptor = g_variant_ref_sink (g_variant_new ("(@a{sv}t@ay@ay@" OSTREE_COMMIT_GVARIANT_STRING "@ay" "a" OSTREE_STATIC_DELTA_META_ENTRY_FORMAT "@a" OSTREE_STATIC_DELTA_FALLBACK_FORMAT ")", g_variant_builder_end (&metadata_builder), @@ -1540,7 +1541,7 @@ ostree_repo_static_delta_generate (OstreeRepo *self, from_csum_v, to_csum_v, to_commit, - g_variant_builder_new (G_VARIANT_TYPE ("ay")), + ot_gvariant_new_bytearray ((guchar*)"", 0), part_headers, fallback_headers)); g_date_time_unref (now); From ab0400b722cb1c9df320bb5231903b1f9a40172b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 21:12:23 -0500 Subject: [PATCH 20/55] [ASAN] deltas: Fix minor memory leak We were leaking the checksum, ensure we free it in both normal and error paths. Closes: #598 Approved by: jlebon --- src/libostree/ostree-repo-static-delta-processing.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index eabe3925..ff5a8a1a 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -286,6 +286,7 @@ _ostree_static_delta_part_execute (OstreeRepo *repo, ret = TRUE; out: + g_clear_pointer (&state->content_checksum, g_checksum_free); return ret; } @@ -941,6 +942,7 @@ dispatch_close (OstreeRepo *repo, goto out; g_clear_pointer (&state->xattrs, g_variant_unref); + g_clear_pointer (&state->content_checksum, g_checksum_free); g_clear_object (&state->content_out); state->checksum_index++; From a1224e2de7eb75b4202494fca2b8a8623383fba0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 21:12:53 -0500 Subject: [PATCH 21/55] [ASAN] cmdline: Fix minor leak in delta cmdline entrypoint Small, but it's important to stay clean. Closes: #598 Approved by: jlebon --- src/ostree/ot-builtin-static-delta.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ostree/ot-builtin-static-delta.c b/src/ostree/ot-builtin-static-delta.c index ca29911a..e1c3bb48 100644 --- a/src/ostree/ot-builtin-static-delta.c +++ b/src/ostree/ot-builtin-static-delta.c @@ -336,11 +336,13 @@ ot_static_delta_builtin_generate (int argc, char **argv, GCancellable *cancellab g_print ("Generating static delta:\n"); g_print (" From: %s\n", from_resolved ? from_resolved : "empty"); g_print (" To: %s\n", to_resolved); - if (!ostree_repo_static_delta_generate (repo, OSTREE_STATIC_DELTA_GENERATE_OPT_MAJOR, - from_resolved, to_resolved, NULL, - g_variant_builder_end (parambuilder), - cancellable, error)) - goto out; + { g_autoptr(GVariant) params = g_variant_ref_sink (g_variant_builder_end (parambuilder)); + if (!ostree_repo_static_delta_generate (repo, OSTREE_STATIC_DELTA_GENERATE_OPT_MAJOR, + from_resolved, to_resolved, NULL, + params, + cancellable, error)) + goto out; + } } From 3a459bac2d25139cfb7810c17ed8eb4702160f57 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 21:14:47 -0500 Subject: [PATCH 22/55] traverse: Use g_hash_table_add And "move semantics" via `g_steal_pointer()`. Just a minor code cleanup I noticed when I was hunting for a leak, which ended up being elsewhere. Closes: #598 Approved by: jlebon --- src/libostree/ostree-repo-traverse.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-repo-traverse.c b/src/libostree/ostree-repo-traverse.c index 46f83240..d125f01a 100644 --- a/src/libostree/ostree-repo-traverse.c +++ b/src/libostree/ostree-repo-traverse.c @@ -349,8 +349,7 @@ traverse_iter (OstreeRepo *repo, g_debug ("Found file object %s", checksum); key = g_variant_ref_sink (ostree_object_name_serialize (checksum, OSTREE_OBJECT_TYPE_FILE)); - g_hash_table_replace (inout_reachable, key, key); - key = NULL; + g_hash_table_add (inout_reachable, g_steal_pointer (&key)); } else if (iterres == OSTREE_REPO_COMMIT_ITER_RESULT_DIR) { @@ -364,14 +363,12 @@ traverse_iter (OstreeRepo *repo, g_debug ("Found dirtree object %s", content_checksum); g_debug ("Found dirmeta object %s", meta_checksum); key = g_variant_ref_sink (ostree_object_name_serialize (meta_checksum, OSTREE_OBJECT_TYPE_DIR_META)); - g_hash_table_replace (inout_reachable, key, key); - key = NULL; + g_hash_table_add (inout_reachable, g_steal_pointer (&key)); key = g_variant_ref_sink (ostree_object_name_serialize (content_checksum, OSTREE_OBJECT_TYPE_DIR_TREE)); if (!g_hash_table_lookup (inout_reachable, key)) { - g_hash_table_replace (inout_reachable, key, key); - key = NULL; + g_hash_table_add (inout_reachable, g_steal_pointer (&key)); if (!traverse_dirtree (repo, content_checksum, inout_reachable, ignore_missing_dirs, cancellable, error)) From 1bb6e514863d2faaa387d4609398c6043504d44b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 22:00:01 -0500 Subject: [PATCH 23/55] [ASAN] sysroot: Fix leak/double free of keyfile origin Use autoptr rather than manual cleanup. The double free isn't a security problem, since we trust origin files. Closes: #598 Approved by: jlebon --- src/libostree/ostree-deployment.c | 2 +- src/libostree/ostree-sysroot.c | 6 +----- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-deployment.c b/src/libostree/ostree-deployment.c index 7b93e6cc..67e896bf 100644 --- a/src/libostree/ostree-deployment.c +++ b/src/libostree/ostree-deployment.c @@ -133,7 +133,6 @@ OstreeDeployment * ostree_deployment_clone (OstreeDeployment *self) { glnx_unref_object OstreeBootconfigParser *new_bootconfig = NULL; - GKeyFile *new_origin = NULL; OstreeDeployment *ret = ostree_deployment_new (self->index, self->osname, self->csum, self->deployserial, self->bootcsum, self->bootserial); @@ -143,6 +142,7 @@ ostree_deployment_clone (OstreeDeployment *self) if (self->origin) { + g_autoptr(GKeyFile) new_origin = NULL; g_autofree char *data = NULL; gsize len; gboolean success; diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 608d4cff..70ce1567 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -613,8 +613,6 @@ parse_origin (OstreeSysroot *self, out: if (error) g_prefix_error (error, "Parsing %s: ", origin_path); - if (ret_origin) - g_key_file_unref (ret_origin); return ret; } @@ -689,7 +687,7 @@ parse_deployment (OstreeSysroot *self, glnx_fd_close int deployment_dfd = -1; const char *deploy_basename; g_autofree char *treebootserial_target = NULL; - GKeyFile *origin = NULL; + g_autoptr(GKeyFile) origin = NULL; g_autofree char *unlocked_development_path = NULL; struct stat stbuf; @@ -751,8 +749,6 @@ parse_deployment (OstreeSysroot *self, if (out_deployment) *out_deployment = g_steal_pointer (&ret_deployment); out: - if (origin) - g_key_file_unref (origin); return ret; } From d83d5b4a29924f208c6955a0917a152d07e4b4d6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 22:01:33 -0500 Subject: [PATCH 24/55] [ASAN] metalink: Fix leaks of buffer We should be religious about the "only set output variables on success", otherwise it makes leaks more likely. But the real leak was us simply not using autoptr in one place. Closes: #598 Approved by: jlebon --- src/libostree/ostree-metalink.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libostree/ostree-metalink.c b/src/libostree/ostree-metalink.c index ad3d5bf9..b3c8b15a 100644 --- a/src/libostree/ostree-metalink.c +++ b/src/libostree/ostree-metalink.c @@ -479,7 +479,7 @@ try_one_url (OstreeMetalinkRequest *self, ret = TRUE; if (out_data) - *out_data = g_bytes_ref (bytes); + *out_data = g_steal_pointer (&bytes); out: return ret; } @@ -492,6 +492,7 @@ try_metalink_targets (OstreeMetalinkRequest *self, { gboolean ret = FALSE; SoupURI *target_uri = NULL; + g_autoptr(GBytes) ret_data = NULL; if (!self->found_a_file_element) { @@ -546,7 +547,7 @@ try_metalink_targets (OstreeMetalinkRequest *self, target_uri = self->urls->pdata[self->current_url_index]; - if (try_one_url (self, target_uri, out_data, &temp_error)) + if (try_one_url (self, target_uri, &ret_data, &temp_error)) break; else { @@ -568,6 +569,8 @@ try_metalink_targets (OstreeMetalinkRequest *self, ret = TRUE; if (out_target_uri) *out_target_uri = soup_uri_copy (target_uri); + if (out_data) + *out_data = g_steal_pointer (&ret_data); out: return ret; } @@ -599,7 +602,7 @@ _ostree_metalink_request_sync (OstreeMetalink *self, gboolean ret = FALSE; OstreeMetalinkRequest request = { 0, }; g_autoptr(GMainContext) mainctx = NULL; - GBytes *out_contents = NULL; + g_autoptr(GBytes) contents = NULL; gsize len; const guint8 *data; @@ -614,13 +617,13 @@ _ostree_metalink_request_sync (OstreeMetalink *self, self->uri, FALSE, FALSE, - &out_contents, + &contents, self->max_size, cancellable, error)) goto out; - data = g_bytes_get_data (out_contents, &len); + data = g_bytes_get_data (contents, &len); if (!g_markup_parse_context_parse (request.parser, (const char*)data, len, error)) goto out; From 161193966c8ecc45c9c0192aba969d0a4a338191 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 22:02:42 -0500 Subject: [PATCH 25/55] [ASAN] bootconfig: Drop a pointless strdup in parser Not entirely sure how this was leaking, but anyways it showed up in ASAN, and it's pointless to strdup here. Closes: #598 Approved by: jlebon --- src/libostree/ostree-bootconfig-parser.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/libostree/ostree-bootconfig-parser.c b/src/libostree/ostree-bootconfig-parser.c index f7728e42..a2ac1072 100644 --- a/src/libostree/ostree-bootconfig-parser.c +++ b/src/libostree/ostree-bootconfig-parser.c @@ -28,7 +28,7 @@ struct _OstreeBootconfigParser GObject parent_instance; gboolean parsed; - char *separators; + const char *separators; GHashTable *options; GPtrArray *lines; @@ -235,7 +235,6 @@ ostree_bootconfig_parser_finalize (GObject *object) g_hash_table_unref (self->options); g_ptr_array_unref (self->lines); - g_free (self->separators); G_OBJECT_CLASS (ostree_bootconfig_parser_parent_class)->finalize (object); } @@ -261,6 +260,6 @@ ostree_bootconfig_parser_new (void) OstreeBootconfigParser *self = NULL; self = g_object_new (OSTREE_TYPE_BOOTCONFIG_PARSER, NULL); - self->separators = g_strdup (" \t"); + self->separators = " \t"; return self; } From 667b734c792a50ca5b257e9ecbc270ba7e65a814 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 22:03:24 -0500 Subject: [PATCH 26/55] [ASAN] set-origin: Squash a leak Just a minor leak in the commandline. Closes: #598 Approved by: jlebon --- src/ostree/ot-admin-builtin-set-origin.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ostree/ot-admin-builtin-set-origin.c b/src/ostree/ot-admin-builtin-set-origin.c index 0e79ab5e..2b6866cc 100644 --- a/src/ostree/ot-admin-builtin-set-origin.c +++ b/src/ostree/ot-admin-builtin-set-origin.c @@ -50,7 +50,7 @@ ot_admin_builtin_set_origin (int argc, char **argv, GCancellable *cancellable, G const char *branch = NULL; glnx_unref_object OstreeRepo *repo = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; - OstreeDeployment *target_deployment = NULL; + glnx_unref_object OstreeDeployment *target_deployment = NULL; context = g_option_context_new ("REMOTENAME URL [BRANCH]"); @@ -85,6 +85,8 @@ ot_admin_builtin_set_origin (int argc, char **argv, GCancellable *cancellable, G "Not currently booted into an OSTree system"); goto out; } + /* To match the below */ + target_deployment = g_object_ref (target_deployment); } else { From 62b94635bc54bb5725c7b12a551427bbcb56c207 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 28 Nov 2016 22:03:53 -0500 Subject: [PATCH 27/55] [ASAN] tests: Fix leaks Just for cleaner sanitizer output. Closes: #598 Approved by: jlebon --- tests/test-basic-c.c | 1 - tests/test-keyfile-utils.c | 17 ++++++++++++++--- tests/test-libarchive-import.c | 5 +++-- tests/test-pull-c.c | 1 + tests/test-rollsum.c | 4 ++-- 5 files changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/test-basic-c.c b/tests/test-basic-c.c index 447c46ea..8e7882b9 100644 --- a/tests/test-basic-c.c +++ b/tests/test-basic-c.c @@ -73,7 +73,6 @@ test_raw_file_to_archive_z2_stream (gconstpointer data) &commit_checksum, &error); g_assert_no_error (error); - reachable = ostree_repo_traverse_new_reachable (); ostree_repo_traverse_commit (repo, commit_checksum, -1, diff --git a/tests/test-keyfile-utils.c b/tests/test-keyfile-utils.c index 4bf37578..8fd59163 100644 --- a/tests/test-keyfile-utils.c +++ b/tests/test-keyfile-utils.c @@ -31,7 +31,7 @@ static GKeyFile *g_keyfile; static void test_get_boolean_with_default (void) { - GError *error = NULL; + g_autoptr(GError) error = NULL; gboolean out = FALSE; GLogLevelFlags always_fatal_mask; @@ -46,18 +46,21 @@ test_get_boolean_with_default (void) FALSE, &out, &error)); + g_clear_error (&error); g_assert_false (ot_keyfile_get_boolean_with_default (g_keyfile, NULL, "a_boolean_true", FALSE, &out, &error)); + g_clear_error (&error); g_assert_false (ot_keyfile_get_boolean_with_default (g_keyfile, "section", NULL, FALSE, &out, &error)); + g_clear_error (&error); /* Restore the old mask. */ g_log_set_always_fatal (always_fatal_mask); @@ -86,6 +89,7 @@ test_get_boolean_with_default (void) &error)); g_assert_true (out); + g_clear_error (&error); g_assert_false (ot_keyfile_get_boolean_with_default (g_keyfile, "a_fake_section", "a_boolean_true", @@ -97,8 +101,8 @@ test_get_boolean_with_default (void) static void test_get_value_with_default (void) { - GError *error = NULL; - char *out = NULL; + g_autoptr(GError) error = NULL; + g_autofree char *out = NULL; GLogLevelFlags always_fatal_mask; const char *section = "section"; @@ -112,18 +116,21 @@ test_get_value_with_default (void) "none", &out, &error)); + g_clear_pointer (&out, g_free); g_assert_false (ot_keyfile_get_value_with_default (g_keyfile, section, NULL, "none", &out, &error)); + g_clear_pointer (&out, g_free); g_assert_false (ot_keyfile_get_value_with_default (g_keyfile, section, NULL, "something", &out, &error)); + g_clear_pointer (&out, g_free); /* Restore the old mask. */ g_log_set_always_fatal (always_fatal_mask); @@ -135,6 +142,7 @@ test_get_value_with_default (void) &out, &error)); g_assert_cmpstr (out, ==, "foo"); + g_clear_pointer (&out, g_free); g_assert (ot_keyfile_get_value_with_default (g_keyfile, section, @@ -143,6 +151,7 @@ test_get_value_with_default (void) &out, &error)); g_assert_cmpstr (out, ==, "correct"); + g_clear_pointer (&out, g_free); g_assert_false (ot_keyfile_get_value_with_default (g_keyfile, "a_fake_section", @@ -150,6 +159,8 @@ test_get_value_with_default (void) "no value", &out, &error)); + g_clear_error (&error); + g_clear_pointer (&out, g_free); } static void diff --git a/tests/test-libarchive-import.c b/tests/test-libarchive-import.c index 05c5568d..254d4143 100644 --- a/tests/test-libarchive-import.c +++ b/tests/test-libarchive-import.c @@ -180,7 +180,7 @@ static void test_libarchive_autocreate_empty (gconstpointer data) { TestData *td = (void*)data; - GError *error = NULL; + g_autoptr(GError) error = NULL; struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; glnx_unref_object OstreeMutableTree *mtree = ostree_mutable_tree_new (); @@ -198,7 +198,7 @@ static void test_libarchive_error_device_file (gconstpointer data) { TestData *td = (void*)data; - GError *error = NULL; + g_autoptr(GError) error = NULL; struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; glnx_unref_object OstreeMutableTree *mtree = ostree_mutable_tree_new (); @@ -563,6 +563,7 @@ int main (int argc, char **argv) r = g_test_run(); + g_clear_object (&td.repo); if (td.tmpd && g_getenv ("TEST_SKIP_CLEANUP") == NULL) (void) glnx_shutil_rm_rf_at (AT_FDCWD, td.tmpd, NULL, NULL); g_free (td.tmpd); diff --git a/tests/test-pull-c.c b/tests/test-pull-c.c index d784312f..43d5d61d 100644 --- a/tests/test-pull-c.c +++ b/tests/test-pull-c.c @@ -127,6 +127,7 @@ int main (int argc, char **argv) g_test_add_data_func ("/test-pull-c/multi-ok-error-repeat", &td, test_pull_multi_error_then_ok); r = g_test_run(); + g_clear_object (&td.repo); return r; } diff --git a/tests/test-rollsum.c b/tests/test-rollsum.c index 97aeb0ae..1ed99645 100644 --- a/tests/test-rollsum.c +++ b/tests/test-rollsum.c @@ -74,8 +74,8 @@ test_rollsum (void) #define MAX_BUFFER_SIZE 1000000 gsize i; int len; - unsigned char *a = malloc (MAX_BUFFER_SIZE); - unsigned char *b = malloc (MAX_BUFFER_SIZE); + g_autofree unsigned char *a = malloc (MAX_BUFFER_SIZE); + g_autofree unsigned char *b = malloc (MAX_BUFFER_SIZE); g_autoptr(GRand) rand = g_rand_new (); /* These two buffers produce the same crc32. */ From 8162463956acc96b5e4e7b80a6ffc0a96d92ff80 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Wed, 30 Nov 2016 10:01:05 +0000 Subject: [PATCH 28/55] tests: prepend to an existing LD_LIBRARY_PATH, GI_TYPELIB_PATH If we're using LD_LIBRARY_PATH for some locally-built library, we want to search those after OSTree's own libraries. Signed-off-by: Simon McVittie Closes: #606 Approved by: cgwalters --- Makefile-tests.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 03e50b9b..517f713b 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -28,8 +28,8 @@ EXTRA_DIST += \ # include the builddir in $PATH so we find our just-built ostree # binary. TESTS_ENVIRONMENT += OT_TESTS_DEBUG=1 \ - GI_TYPELIB_PATH=$$(cd $(top_builddir) && pwd) \ - LD_LIBRARY_PATH=$$(cd $(top_builddir)/.libs && pwd) \ + GI_TYPELIB_PATH=$$(cd $(top_builddir) && pwd)$${GI_TYPELIB_PATH:+:$$GI_TYPELIB_PATH} \ + LD_LIBRARY_PATH=$$(cd $(top_builddir)/.libs && pwd)$${LD_LIBRARY_PATH:+:$${LD_LIBRARY_PATH}} \ PATH=$$(cd $(top_builddir)/tests && pwd):$${PATH} \ $(NULL) From c733e21a84f788cb4c9ac4e723743cedb898b345 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Thu, 1 Dec 2016 16:28:27 +0000 Subject: [PATCH 29/55] Terminate individual tests after (10 * $TEST_TIMEOUT_FACTOR) minutes While using the Automake parallel test harness, if a test hangs for long enough for an external watchdog to kill the entire build process (as happens in Debian sbuild after 150 minutes with no activity on stdout/stderr), the logs will not be shown. If we make an individual test time out sooner, logs are more likely to be shown. We use SIGABRT so that the process(es) under test will dump core, allowing the point at which ostree is blocking to be analyzed. After 1 minute, if any have not died, we kill them again with SIGKILL. To support slow platforms and slow debugging tools, if TEST_TIMEOUT_FACTOR is set, multiply the 10 minute timeout by that. Signed-off-by: Simon McVittie Closes: #607 Approved by: cgwalters --- buildutil/tap-test | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/buildutil/tap-test b/buildutil/tap-test index c8da31e7..ac729d8d 100755 --- a/buildutil/tap-test +++ b/buildutil/tap-test @@ -19,7 +19,11 @@ function skip_cleanup() { echo "Skipping cleanup of ${tempdir}" } cd ${tempdir} -${srcd}/${bn} -k --tap +timeout \ + --kill-after=60 \ + --signal=ABRT \ + $(( 600 * ${TEST_TIMEOUT_FACTOR:-1} )) \ + ${srcd}/${bn} -k --tap rc=$? case "${TEST_SKIP_CLEANUP:-}" in no|"") cleanup ;; From 7bb0ff46a438977297329ad6949faa4fae06f543 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 2 Dec 2016 13:34:32 -0500 Subject: [PATCH 30/55] Define and use cleanup helpers for libarchive This should fix some of the ASAN leaks around libarchive usage, and is generally better. Closes: #609 Approved by: jlebon --- Makefile-libostree.am | 1 + .../ostree-libarchive-input-stream.h | 1 + src/libostree/ostree-libarchive-private.h | 42 +++++++++++++++++++ src/libostree/ostree-repo-libarchive.c | 4 +- src/ostree/ot-builtin-export.c | 7 +--- tests/test-libarchive-import.c | 27 ++++++------ tests/test-ot-opt-utils.c | 1 + 7 files changed, 61 insertions(+), 22 deletions(-) create mode 100644 src/libostree/ostree-libarchive-private.h diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 0be60a18..d6ae6023 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -134,6 +134,7 @@ libostree_1_la_SOURCES = \ if USE_LIBARCHIVE libostree_1_la_SOURCES += src/libostree/ostree-libarchive-input-stream.h \ src/libostree/ostree-libarchive-input-stream.c \ + src/libostree/ostree-libarchive-private.h \ $(NULL) endif if HAVE_LIBSOUP_CLIENT_CERTS diff --git a/src/libostree/ostree-libarchive-input-stream.h b/src/libostree/ostree-libarchive-input-stream.h index bcfc7bd2..7f88693e 100644 --- a/src/libostree/ostree-libarchive-input-stream.h +++ b/src/libostree/ostree-libarchive-input-stream.h @@ -23,6 +23,7 @@ #pragma once #include +#include "ostree-libarchive-private.h" G_BEGIN_DECLS diff --git a/src/libostree/ostree-libarchive-private.h b/src/libostree/ostree-libarchive-private.h new file mode 100644 index 00000000..177c6568 --- /dev/null +++ b/src/libostree/ostree-libarchive-private.h @@ -0,0 +1,42 @@ +/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*- + * + * Copyright (C) 2016 Colin Walters + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General + * Public License along with this library; if not, write to the + * Free Software Foundation, Inc., 59 Temple Place, Suite 330, + * Boston, MA 02111-1307, USA. + * + * Author: Alexander Larsson + */ + +#pragma once + +#include +#include "libglnx.h" +#ifdef HAVE_LIBARCHIVE +#include +#include +#endif + +G_BEGIN_DECLS + +#ifdef HAVE_LIBARCHIVE +GLNX_DEFINE_CLEANUP_FUNCTION (void *, flatpak_local_free_write_archive, archive_write_free) +#define ot_cleanup_write_archive __attribute__((cleanup (flatpak_local_free_write_archive))) + +GLNX_DEFINE_CLEANUP_FUNCTION (void *, flatpak_local_free_read_archive, archive_read_free) +#define ot_cleanup_read_archive __attribute__((cleanup (flatpak_local_free_read_archive))) +#endif + +G_END_DECLS diff --git a/src/libostree/ostree-repo-libarchive.c b/src/libostree/ostree-repo-libarchive.c index 02a1180a..01982894 100644 --- a/src/libostree/ostree-repo-libarchive.c +++ b/src/libostree/ostree-repo-libarchive.c @@ -913,10 +913,9 @@ ostree_repo_write_archive_to_mtree (OstreeRepo *self, { #ifdef HAVE_LIBARCHIVE gboolean ret = FALSE; - struct archive *a = NULL; + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; - a = archive_read_new (); #ifdef HAVE_ARCHIVE_READ_SUPPORT_FILTER_ALL archive_read_support_filter_all (a); #else @@ -945,7 +944,6 @@ ostree_repo_write_archive_to_mtree (OstreeRepo *self, if (a) { (void)archive_read_close (a); - (void)archive_read_free (a); } return ret; #else diff --git a/src/ostree/ot-builtin-export.c b/src/ostree/ot-builtin-export.c index db4dbc31..b0bb40fa 100644 --- a/src/ostree/ot-builtin-export.c +++ b/src/ostree/ot-builtin-export.c @@ -24,6 +24,7 @@ #include "ot-builtins.h" #include "ostree.h" #include "ostree-repo-file.h" +#include "ostree-libarchive-private.h" #include "otutil.h" #ifdef HAVE_LIBARCHIVE @@ -67,7 +68,7 @@ ostree_builtin_export (int argc, char **argv, GCancellable *cancellable, GError g_autoptr(GFile) subtree = NULL; g_autofree char *commit = NULL; g_autoptr(GVariant) commit_data = NULL; - struct archive *a = NULL; + ot_cleanup_write_archive struct archive *a = NULL; OstreeRepoExportArchiveOptions opts = { 0, }; context = g_option_context_new ("COMMIT - Stream COMMIT to stdout in tar format"); @@ -154,10 +155,6 @@ ostree_builtin_export (int argc, char **argv, GCancellable *cancellable, GError ret = TRUE; out: -#ifdef HAVE_LIBARCHIVE - if (a) - archive_write_free (a); -#endif if (context) g_option_context_free (context); return ret; diff --git a/tests/test-libarchive-import.c b/tests/test-libarchive-import.c index 254d4143..e9be7f66 100644 --- a/tests/test-libarchive-import.c +++ b/tests/test-libarchive-import.c @@ -26,6 +26,7 @@ #include #include +#include "ostree-libarchive-private.h" #include #include @@ -40,7 +41,7 @@ static void test_data_init (TestData *td) { GError *error = NULL; - struct archive *a = archive_write_new (); + ot_cleanup_write_archive struct archive *a = archive_write_new (); struct archive_entry *ae; uid_t uid = getuid (); gid_t gid = getgid (); @@ -115,12 +116,12 @@ test_data_init (TestData *td) archive_entry_free (ae); g_assert_cmpint (ARCHIVE_OK, ==, archive_write_close (a)); - g_assert_cmpint (ARCHIVE_OK, ==, archive_write_free (a)); td->fd_empty = openat (AT_FDCWD, "empty.tar.gz", O_CREAT | O_EXCL | O_RDWR | O_CLOEXEC, 0644); g_assert (td->fd_empty >= 0); (void) unlink ("empty.tar.gz"); + g_assert_cmpint (ARCHIVE_OK, ==, archive_write_free (a)); a = archive_write_new (); g_assert (a); @@ -128,7 +129,6 @@ test_data_init (TestData *td) g_assert_cmpint (0, ==, archive_write_add_filter_gzip (a)); g_assert_cmpint (0, ==, archive_write_open_fd (a, td->fd_empty)); g_assert_cmpint (ARCHIVE_OK, ==, archive_write_close (a)); - g_assert_cmpint (ARCHIVE_OK, ==, archive_write_free (a)); { g_autoptr(GFile) repopath = g_file_new_for_path ("repo"); td->repo = ostree_repo_new (repopath); @@ -165,7 +165,7 @@ test_libarchive_noautocreate_empty (gconstpointer data) { TestData *td = (void*)data; GError *error = NULL; - struct archive *a = archive_read_new (); + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; glnx_unref_object OstreeMutableTree *mtree = ostree_mutable_tree_new (); @@ -181,7 +181,7 @@ test_libarchive_autocreate_empty (gconstpointer data) { TestData *td = (void*)data; g_autoptr(GError) error = NULL; - struct archive *a = archive_read_new (); + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; glnx_unref_object OstreeMutableTree *mtree = ostree_mutable_tree_new (); @@ -199,7 +199,7 @@ test_libarchive_error_device_file (gconstpointer data) { TestData *td = (void*)data; g_autoptr(GError) error = NULL; - struct archive *a = archive_read_new (); + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; glnx_unref_object OstreeMutableTree *mtree = ostree_mutable_tree_new (); @@ -207,6 +207,7 @@ test_libarchive_error_device_file (gconstpointer data) (void)ostree_repo_import_archive_to_mtree (td->repo, &opts, a, mtree, NULL, NULL, &error); g_assert (error != NULL); + g_clear_error (&error); } static gboolean @@ -268,8 +269,8 @@ static void test_libarchive_ignore_device_file (gconstpointer data) { TestData *td = (void*)data; - GError *error = NULL; - struct archive *a = archive_read_new (); + g_autoptr(GError) error = NULL; + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; if (skip_if_no_xattr (td)) @@ -331,7 +332,7 @@ test_libarchive_ostree_convention (gconstpointer data) { TestData *td = (void*)data; GError *error = NULL; - struct archive *a = archive_read_new (); + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; if (skip_if_no_xattr (td)) @@ -373,7 +374,7 @@ test_libarchive_xattr_callback (gconstpointer data) { TestData *td = (void*)data; GError *error = NULL; - struct archive *a = archive_read_new (); + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0 }; OstreeRepoCommitModifier *modifier = NULL; char buf[7] = { 0 }; @@ -428,7 +429,7 @@ static void entry_pathname_test_helper (gconstpointer data, gboolean on) { TestData *td = (void*)data; GError *error = NULL; - struct archive *a = archive_read_new (); + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0, }; OstreeRepoCommitModifier *modifier = NULL; gboolean met_etc_file = FALSE; @@ -468,7 +469,6 @@ entry_pathname_test_helper (gconstpointer data, gboolean on) goto out; } - archive_read_free (a); ostree_repo_commit_modifier_unref (modifier); out: g_assert_no_error (error); @@ -491,7 +491,7 @@ test_libarchive_selinux (gconstpointer data) { TestData *td = (void*)data; GError *error = NULL; - struct archive *a = archive_read_new (); + ot_cleanup_read_archive struct archive *a = archive_read_new (); OstreeRepoImportArchiveOptions opts = { 0 }; glnx_unref_object OstreeSePolicy *sepol = NULL; OstreeRepoCommitModifier *modifier = NULL; @@ -536,7 +536,6 @@ test_libarchive_selinux (gconstpointer data) g_assert_cmpstr (buf, ==, "system_u:object_r:etc_t:s0"); out: - archive_read_free (a); if (modifier) ostree_repo_commit_modifier_unref (modifier); g_assert_no_error (error); diff --git a/tests/test-ot-opt-utils.c b/tests/test-ot-opt-utils.c index 7f334c36..1a7230b3 100644 --- a/tests/test-ot-opt-utils.c +++ b/tests/test-ot-opt-utils.c @@ -25,6 +25,7 @@ #include #include #include "ot-opt-utils.h" +#include "libglnx.h" static GString *printerr_str = NULL; From e1c48c8adbd14f8767d44d46b1a7a5f896ab1129 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 2 Dec 2016 13:45:04 -0500 Subject: [PATCH 31/55] [ASAN] tests: Cleanup all current remaining leaks We now run fully through ASAN here. Closes: #609 Approved by: jlebon --- tests/test-bsdiff.c | 4 +++- tests/test-lzma.c | 7 ++++--- tests/test-mutable-tree.c | 10 ++++++---- tests/test-ot-opt-utils.c | 6 +++--- tests/test-ot-tool-util.c | 7 +++++-- tests/test-ot-unix-utils.c | 13 ++++++++++--- tests/test-varint.c | 2 +- 7 files changed, 32 insertions(+), 17 deletions(-) diff --git a/tests/test-bsdiff.c b/tests/test-bsdiff.c index 5c5eb104..24772a1e 100644 --- a/tests/test-bsdiff.c +++ b/tests/test-bsdiff.c @@ -89,7 +89,9 @@ test_bsdiff (void) g_output_stream_close (out, NULL, NULL); /* Now generate NEW_GENERATED from OLD and OUT. */ - in = g_memory_input_stream_new_from_bytes (g_memory_output_stream_steal_as_bytes (G_MEMORY_OUTPUT_STREAM (out))); + { g_autoptr(GBytes) bytes = g_memory_output_stream_steal_as_bytes (G_MEMORY_OUTPUT_STREAM (out)); + in = g_memory_input_stream_new_from_bytes (bytes); + } bspatch_stream.read = bzpatch_read; bspatch_stream.opaque = in; diff --git a/tests/test-lzma.c b/tests/test-lzma.c index 8d37d47f..1f7d2559 100644 --- a/tests/test-lzma.c +++ b/tests/test-lzma.c @@ -32,7 +32,7 @@ static void helper_test_compress_decompress (const guint8 *data, gssize data_size) { - GError *error = NULL; + g_autoptr(GError) error = NULL; g_autoptr(GOutputStream) out_compress = g_memory_output_stream_new_resizable (); g_autoptr(GOutputStream) out_decompress = NULL; g_autoptr(GInputStream) in_compress = g_memory_input_stream_new_from_data (data, data_size, NULL); @@ -55,9 +55,10 @@ helper_test_compress_decompress (const guint8 *data, gssize data_size) { gssize n_bytes_written; g_autoptr(GInputStream) convin = NULL; - g_autoptr(GConverter) decompressor = (GConverter*)_ostree_lzma_decompressor_new (); + g_autoptr(GConverter) decompressor = (GConverter*)_ostree_lzma_decompressor_new (); + g_autoptr(GBytes) bytes = g_memory_output_stream_steal_as_bytes (G_MEMORY_OUTPUT_STREAM (out_compress)); - in_decompress = g_memory_input_stream_new_from_bytes (g_memory_output_stream_steal_as_bytes (G_MEMORY_OUTPUT_STREAM (out_compress))); + in_decompress = g_memory_input_stream_new_from_bytes (bytes); convin = g_converter_input_stream_new ((GInputStream*) in_decompress, decompressor); n_bytes_written = g_output_stream_splice (out_decompress, convin, G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET | G_OUTPUT_STREAM_SPLICE_CLOSE_SOURCE, diff --git a/tests/test-mutable-tree.c b/tests/test-mutable-tree.c index b357f691..9df36ac6 100644 --- a/tests/test-mutable-tree.c +++ b/tests/test-mutable-tree.c @@ -84,7 +84,7 @@ test_ensure_parent_dirs (void) glnx_unref_object OstreeMutableTree *tree = ostree_mutable_tree_new (); glnx_unref_object OstreeMutableTree *parent = NULL; g_autoptr(GPtrArray) split_path = NULL; - GError *error = NULL; + g_autoptr(GError) error = NULL; const char *pathname = "/foo/bar/baz"; const char *checksum = "01234567890123456789012345678901"; g_autofree char *source_checksum = NULL; @@ -103,6 +103,7 @@ test_ensure_parent_dirs (void) g_assert_false (ostree_mutable_tree_lookup (tree, "bar", &source_checksum2, &source_subdir2, &error)); + g_clear_error (&error); } static void @@ -110,7 +111,7 @@ test_ensure_dir (void) { glnx_unref_object OstreeMutableTree *tree = ostree_mutable_tree_new (); glnx_unref_object OstreeMutableTree *parent = NULL; - GError *error = NULL; + g_autoptr(GError) error = NULL; const char *dirname = "foo"; const char *filename = "bar"; const char *checksum = "01234567890123456789012345678901"; @@ -122,13 +123,14 @@ test_ensure_dir (void) g_assert (ostree_mutable_tree_replace_file (tree, filename, checksum, &error)); g_assert_false (ostree_mutable_tree_ensure_dir (tree, filename, &parent, &error)); + g_clear_error (&error); } static void test_replace_file (void) { glnx_unref_object OstreeMutableTree *tree = ostree_mutable_tree_new (); - GError *error = NULL; + g_autoptr(GError) error = NULL; const char *filename = "bar"; const char *checksum = "01234567890123456789012345678901"; const char *checksum2 = "ABCDEF01234567890123456789012345"; @@ -157,7 +159,7 @@ test_contents_checksum (void) const char *subdir_checksum = "ABCD0123456789012345678901234567"; glnx_unref_object OstreeMutableTree *tree = ostree_mutable_tree_new (); glnx_unref_object OstreeMutableTree *subdir = NULL; - GError *error = NULL; + g_autoptr(GError) error = NULL; g_assert_null (ostree_mutable_tree_get_contents_checksum (tree)); ostree_mutable_tree_set_contents_checksum (tree, checksum); diff --git a/tests/test-ot-opt-utils.c b/tests/test-ot-opt-utils.c index 1a7230b3..77ecdf30 100644 --- a/tests/test-ot-opt-utils.c +++ b/tests/test-ot-opt-utils.c @@ -40,19 +40,19 @@ util_usage_error_printerr (const gchar *string) static void test_ot_util_usage_error (void) { - GError *error = NULL; - GOptionContext *context = g_option_context_new ("[TEST]"); + g_autoptr(GError) error = NULL; + g_autoptr(GOptionContext) context = g_option_context_new ("[TEST]"); GPrintFunc old_printerr = g_set_printerr_handler (util_usage_error_printerr); ot_util_usage_error (context, "find_me", &error); g_assert_nonnull (strstr (printerr_str->str, "[TEST]")); g_assert_nonnull (strstr (error->message, "find_me")); + g_clear_error (&error); g_set_printerr_handler (old_printerr); g_string_free (printerr_str, TRUE); printerr_str = NULL; - g_option_context_free (context); } int main (int argc, char **argv) diff --git a/tests/test-ot-tool-util.c b/tests/test-ot-tool-util.c index e63a0185..13030fe4 100644 --- a/tests/test-ot-tool-util.c +++ b/tests/test-ot-tool-util.c @@ -43,7 +43,7 @@ ot_parse_keyvalue (const char *keyvalue, static void test_ot_parse_boolean (void) { - GError *error = NULL; + g_autoptr(GError) error = NULL; gboolean out = FALSE; g_assert_true (ot_parse_boolean ("yes", &out, &error)); g_assert_true (out); @@ -70,15 +70,17 @@ test_ot_parse_boolean (void) out = TRUE; g_assert_true (ot_parse_boolean ("none", &out, &error)); g_assert_false (out); + g_clear_error (&error); g_assert_false (ot_parse_boolean ("FOO", &out, &error)); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED); + g_clear_error (&error); } static void test_ot_parse_keyvalue (void) { - GError *error = NULL; + g_autoptr(GError) error = NULL; char *keyvalue[] = {"foo=bar", "a=", "b=1231231"}; char *key[] = {"foo", "a", "b"}; char *value[] = {"bar", "", "1231231"}; @@ -104,6 +106,7 @@ test_ot_parse_keyvalue (void) &out_value, &error)); g_assert_error (error, G_IO_ERROR, G_IO_ERROR_FAILED); + g_clear_error (&error); } } diff --git a/tests/test-ot-unix-utils.c b/tests/test-ot-unix-utils.c index 6c77d2a2..5eeab1d1 100644 --- a/tests/test-ot-unix-utils.c +++ b/tests/test-ot-unix-utils.c @@ -50,22 +50,29 @@ test_ot_util_path_split_validate (void) static void test_ot_util_filename_validate (void) { - GError *error = NULL; + g_autoptr(GError) error = NULL; /* Check for valid inputs. */ g_assert (ot_util_filename_validate ("valid", &error)); + g_assert_no_error (error); g_assert (ot_util_filename_validate ("valid_file_name", &error)); + g_assert_no_error (error); g_assert (ot_util_filename_validate ("file.name", &error)); + g_assert_no_error (error); g_assert (ot_util_filename_validate ("foo..", &error)); + g_assert_no_error (error); g_assert (ot_util_filename_validate ("..bar", &error)); + g_assert_no_error (error); g_assert (ot_util_filename_validate ("baz:", &error)); + g_assert_no_error (error); /* Check for invalid inputs. */ g_assert_false (ot_util_filename_validate ("not/valid/file/name", &error)); - error = NULL; + g_clear_error (&error); g_assert_false (ot_util_filename_validate (".", &error)); - error = NULL; + g_clear_error (&error); g_assert_false (ot_util_filename_validate ("..", &error)); + g_clear_error (&error); } int main (int argc, char **argv) diff --git a/tests/test-varint.c b/tests/test-varint.c index 76f85573..062c47ef 100644 --- a/tests/test-varint.c +++ b/tests/test-varint.c @@ -27,7 +27,7 @@ static void check_one_roundtrip (guint64 val) { - GString *buf = g_string_new (NULL); + g_autoptr(GString) buf = g_string_new (NULL); guint64 newval; gsize bytes_read; From 555f1fb03d8dd56b36abe46355021665ee558cef Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 5 Dec 2016 12:58:43 -0500 Subject: [PATCH 32/55] tests: Use G_DEBUG=fatal-warnings here too I am trying to track down a warning I see in `test-keyfile-utils` which turned out to be the installed case only, but let's inject this here too. (The GLib default is broken, but it's hard to fix upstream without breaking the world) Closes: #610 Approved by: jlebon --- Makefile-tests.am | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile-tests.am b/Makefile-tests.am index 517f713b..d4685237 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 \ + G_DEBUG=fatal-warnings \ GI_TYPELIB_PATH=$$(cd $(top_builddir) && pwd)$${GI_TYPELIB_PATH:+:$$GI_TYPELIB_PATH} \ LD_LIBRARY_PATH=$$(cd $(top_builddir)/.libs && pwd)$${LD_LIBRARY_PATH:+:$${LD_LIBRARY_PATH}} \ PATH=$$(cd $(top_builddir)/tests && pwd):$${PATH} \ From 66da1199f0e6bff78b94d54b619c1bc2eed009c6 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 5 Dec 2016 13:08:39 -0500 Subject: [PATCH 33/55] tests/keyfile-utils: Drop tests covering preconditions The spam in stderr was bothering me, and further at some eventual point in the future we want to annotate the functions with `__attribute__((nonnull))` which would then cause tests like these to become undefined behavior. The coverage of this isn't worth the log spam basically. Closes: #611 Approved by: jlebon --- tests/test-keyfile-utils.c | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/tests/test-keyfile-utils.c b/tests/test-keyfile-utils.c index 8fd59163..376b0e82 100644 --- a/tests/test-keyfile-utils.c +++ b/tests/test-keyfile-utils.c @@ -34,37 +34,6 @@ test_get_boolean_with_default (void) g_autoptr(GError) error = NULL; gboolean out = FALSE; - GLogLevelFlags always_fatal_mask; - - /* Avoid that g_return_val_if_fail causes the test to fail. */ - always_fatal_mask = g_log_set_always_fatal (0); - - - g_assert_false (ot_keyfile_get_boolean_with_default (NULL, - "section", - "a_boolean_true", - FALSE, - &out, - &error)); - g_clear_error (&error); - g_assert_false (ot_keyfile_get_boolean_with_default (g_keyfile, - NULL, - "a_boolean_true", - FALSE, - &out, - &error)); - g_clear_error (&error); - g_assert_false (ot_keyfile_get_boolean_with_default (g_keyfile, - "section", - NULL, - FALSE, - &out, - &error)); - g_clear_error (&error); - - /* Restore the old mask. */ - g_log_set_always_fatal (always_fatal_mask); - g_assert (ot_keyfile_get_boolean_with_default (g_keyfile, "section", "a_boolean_true", From 4c8fc92aa02dbbd474009b61ca54cf49ef29da56 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 5 Dec 2016 17:22:46 -0500 Subject: [PATCH 34/55] lib: Always checksum content in deltas This is a follow up to conversation on list - in practice, if we're backing away from summary signing, then it makes sense to remove the special casing for checksums in deltas around summary signatures. This is also related to the recent change to enable GPG checking for commits in deltas - now we have a more coherent story between the previous pull path and deltas. I didn't do any performance checking, and while it's slightly annoying that we're now doing sha256 on the delta content twice (once for the part and once per object)...sha256 is pretty fast, I think most users are I/O bound anyways, and it'd drop even farther if we started using openssl. Closes: #612 Approved by: jlebon --- src/libostree/ostree-repo-pull.c | 4 - src/libostree/ostree-repo-static-delta-core.c | 5 +- .../ostree-repo-static-delta-private.h | 2 - .../ostree-repo-static-delta-processing.c | 83 ++++++------------- 4 files changed, 27 insertions(+), 67 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index ffa387a9..80c70438 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1010,8 +1010,6 @@ static_deltapart_fetch_on_complete (GObject *object, _ostree_static_delta_part_execute_async (pull_data->repo, fetch_data->objects, part, - /* Trust checksums if summary was gpg signed */ - pull_data->gpg_verify_summary && pull_data->summary_data_sig, pull_data->cancellable, on_static_delta_written, fetch_data); @@ -1661,7 +1659,6 @@ process_one_static_delta (OtPullData *pull_data, g_autoptr(GBytes) inline_part_bytes = NULL; guint64 size, usize; guint32 version; - const gboolean trusted = pull_data->gpg_verify_summary && pull_data->summary_data_sig; header = g_variant_get_child_value (headers, i); g_variant_get (header, "(u@aytt@ay)", &version, &csum_v, &size, &usize, &objects); @@ -1731,7 +1728,6 @@ process_one_static_delta (OtPullData *pull_data, _ostree_static_delta_part_execute_async (pull_data->repo, fetch_data->objects, inline_delta_part, - trusted, pull_data->cancellable, on_static_delta_written, fetch_data); diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 133ab016..5ef4df90 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -435,8 +435,7 @@ ostree_repo_static_delta_execute_offline (OstreeRepo *self, } if (!_ostree_static_delta_part_execute (self, objects, part, skip_validation, - FALSE, NULL, - cancellable, error)) + NULL, cancellable, error)) { g_prefix_error (error, "Executing delta part %i: ", i); goto out; @@ -653,7 +652,7 @@ show_one_part (OstreeRepo *self, (guint64)g_variant_n_children (ops)); if (!_ostree_static_delta_part_execute (self, objects, - part, TRUE, TRUE, + part, TRUE, &stats, cancellable, error)) goto out; diff --git a/src/libostree/ostree-repo-static-delta-private.h b/src/libostree/ostree-repo-static-delta-private.h index 31e8971e..6121c482 100644 --- a/src/libostree/ostree-repo-static-delta-private.h +++ b/src/libostree/ostree-repo-static-delta-private.h @@ -141,7 +141,6 @@ typedef struct { gboolean _ostree_static_delta_part_execute (OstreeRepo *repo, GVariant *header, GVariant *part_payload, - gboolean trusted, gboolean stats_only, OstreeDeltaExecuteStats *stats, GCancellable *cancellable, @@ -150,7 +149,6 @@ gboolean _ostree_static_delta_part_execute (OstreeRepo *repo, void _ostree_static_delta_part_execute_async (OstreeRepo *repo, GVariant *header, GVariant *part_payload, - gboolean trusted, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data); diff --git a/src/libostree/ostree-repo-static-delta-processing.c b/src/libostree/ostree-repo-static-delta-processing.c index ff5a8a1a..a71e070f 100644 --- a/src/libostree/ostree-repo-static-delta-processing.c +++ b/src/libostree/ostree-repo-static-delta-processing.c @@ -39,7 +39,6 @@ G_STATIC_ASSERT (sizeof (guint) >= sizeof (guint32)); typedef struct { - gboolean trusted; gboolean stats_only; OstreeRepo *repo; guint checksum_index; @@ -180,7 +179,6 @@ gboolean _ostree_static_delta_part_execute (OstreeRepo *repo, GVariant *objects, GVariant *part, - gboolean trusted, gboolean stats_only, OstreeDeltaExecuteStats *stats, GCancellable *cancellable, @@ -200,7 +198,6 @@ _ostree_static_delta_part_execute (OstreeRepo *repo, state->repo = repo; state->async_error = error; - state->trusted = trusted; state->stats_only = stats_only; if (!_ostree_static_delta_parse_checksum_array (objects, @@ -296,7 +293,6 @@ typedef struct { GVariant *part; GCancellable *cancellable; GSimpleAsyncResult *result; - gboolean trusted; } StaticDeltaPartExecuteAsyncData; static void @@ -323,7 +319,6 @@ static_delta_part_execute_thread (GSimpleAsyncResult *res, if (!_ostree_static_delta_part_execute (data->repo, data->header, data->part, - data->trusted, FALSE, NULL, cancellable, &error)) g_simple_async_result_take_error (res, error); @@ -333,7 +328,6 @@ void _ostree_static_delta_part_execute_async (OstreeRepo *repo, GVariant *header, GVariant *part, - gboolean trusted, GCancellable *cancellable, GAsyncReadyCallback callback, gpointer user_data) @@ -344,7 +338,6 @@ _ostree_static_delta_part_execute_async (OstreeRepo *repo, asyncdata->repo = g_object_ref (repo); asyncdata->header = g_variant_ref (header); asyncdata->part = g_variant_ref (part); - asyncdata->trusted = trusted; asyncdata->cancellable = cancellable ? g_object_ref (cancellable) : NULL; asyncdata->result = g_simple_async_result_new ((GObject*) repo, @@ -517,9 +510,9 @@ dispatch_bspatch (OstreeRepo *repo, return ret; } -/* When processing untrusted static deltas, we need to checksum the - * file content, which includes a header. Compare with what - * ostree_checksum_file_from_input() is doing too. +/* Before, we had a distinction between "trusted" and "untrusted" deltas + * which we've decided wasn't a good idea. Now, we always checksum the content. + * Compare with what ostree_checksum_file_from_input() is doing too. */ static gboolean handle_untrusted_content_checksum (OstreeRepo *repo, @@ -530,13 +523,10 @@ handle_untrusted_content_checksum (OstreeRepo *repo, g_autoptr(GVariant) header = NULL; g_autoptr(GFileInfo) finfo = NULL; gsize bytes_written; - - if (state->trusted) - return TRUE; finfo = _ostree_header_gfile_info_new (state->mode, state->uid, state->gid); header = _ostree_file_header_new (finfo, state->xattrs); - + state->content_checksum = g_checksum_new (G_CHECKSUM_SHA256); if (!_ostree_write_variant_with_size (NULL, header, 0, &bytes_written, state->content_checksum, @@ -579,26 +569,16 @@ dispatch_open_splice_and_close (OstreeRepo *repo, metadata = g_variant_new_from_data (ostree_metadata_variant_type (state->output_objtype), state->payload_data + offset, length, TRUE, NULL, NULL); - if (state->trusted) - { - if (!ostree_repo_write_metadata_trusted (state->repo, state->output_objtype, - state->checksum, - metadata, - cancellable, - error)) - goto out; - } - else - { - g_autofree guchar *actual_csum = NULL; + { + g_autofree guchar *actual_csum = NULL; - if (!ostree_repo_write_metadata (state->repo, state->output_objtype, - state->checksum, - metadata, &actual_csum, - cancellable, - error)) - goto out; - } + if (!ostree_repo_write_metadata (state->repo, state->output_objtype, + state->checksum, + metadata, &actual_csum, + cancellable, + error)) + goto out; + } } else { @@ -672,29 +652,18 @@ dispatch_open_splice_and_close (OstreeRepo *repo, &object_input, &objlen, cancellable, error)) goto out; - - if (state->trusted) - { - if (!ostree_repo_write_content_trusted (state->repo, - state->checksum, - object_input, - objlen, - cancellable, - error)) - goto out; - } - else - { - g_autofree guchar *actual_csum = NULL; - if (!ostree_repo_write_content (state->repo, - state->checksum, - object_input, - objlen, - &actual_csum, - cancellable, - error)) - goto out; - } + + { + g_autofree guchar *actual_csum = NULL; + if (!ostree_repo_write_content (state->repo, + state->checksum, + object_input, + objlen, + &actual_csum, + cancellable, + error)) + goto out; + } } } @@ -920,8 +889,6 @@ dispatch_close (OstreeRepo *repo, { const char *actual_checksum = g_checksum_get_string (state->content_checksum); - g_assert (!state->trusted); - if (strcmp (actual_checksum, state->checksum) != 0) { g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, From 0fb3645fedf3596360af1542038109a3f7fa2e6d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 5 Dec 2016 20:56:55 -0500 Subject: [PATCH 35/55] pull: Write .commitpartial for local pulls first too This is what we do for non-local (i.e. HTTP) pulls; we wnat to correctly handle being interrupted during partial pulls. Closes: https://github.com/ostreedev/ostree/issues/579 Closes: #613 Approved by: jlebon --- src/libostree/ostree-repo-pull.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 80c70438..47da6846 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -1273,15 +1273,15 @@ scan_one_metadata_object_c (OtPullData *pull_data, { if (!is_stored) { - if (!ostree_repo_import_object_from_with_trust (pull_data->repo, pull_data->remote_repo_local, - objtype, tmp_checksum, !pull_data->is_untrusted, - cancellable, error)) - goto out; if (objtype == OSTREE_OBJECT_TYPE_COMMIT) { if (!write_commitpartial_for (pull_data, tmp_checksum, error)) goto out; } + if (!ostree_repo_import_object_from_with_trust (pull_data->repo, pull_data->remote_repo_local, + objtype, tmp_checksum, !pull_data->is_untrusted, + cancellable, error)) + goto out; } is_stored = TRUE; is_requested = TRUE; From 247a8718bb699fdc6ae59e00b904f4bc9a2a401f Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 5 Dec 2016 22:04:06 -0500 Subject: [PATCH 36/55] lib: Remove unused ostree_metalink_get_uri() While doing something else I noticed it was unused. Closes: #615 Approved by: jlebon --- src/libostree/ostree-metalink.c | 6 ------ src/libostree/ostree-metalink.h | 2 -- 2 files changed, 8 deletions(-) diff --git a/src/libostree/ostree-metalink.c b/src/libostree/ostree-metalink.c index b3c8b15a..981b30ed 100644 --- a/src/libostree/ostree-metalink.c +++ b/src/libostree/ostree-metalink.c @@ -642,9 +642,3 @@ _ostree_metalink_request_sync (OstreeMetalink *self, g_clear_pointer (&request.parser, g_markup_parse_context_free); return ret; } - -SoupURI * -_ostree_metalink_get_uri (OstreeMetalink *self) -{ - return self->uri; -} diff --git a/src/libostree/ostree-metalink.h b/src/libostree/ostree-metalink.h index 8f63d372..9d09f007 100644 --- a/src/libostree/ostree-metalink.h +++ b/src/libostree/ostree-metalink.h @@ -48,8 +48,6 @@ OstreeMetalink *_ostree_metalink_new (OstreeFetcher *fetcher, guint64 max_size, SoupURI *uri); -SoupURI *_ostree_metalink_get_uri (OstreeMetalink *self); - gboolean _ostree_metalink_request_sync (OstreeMetalink *self, SoupURI **out_target_uri, GBytes **out_data, From 37965644ff37eb415cba1dfc952913da6d655813 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 5 Dec 2016 21:06:43 -0500 Subject: [PATCH 37/55] tree-wide: Use g_hash_table_add() where applicable Just noticed a few while reading some code, decided to do a quick cleanup. It's shorter and clearer. Closes: #614 Approved by: jlebon --- src/libostree/ostree-bootconfig-parser.c | 2 +- src/libostree/ostree-repo-checkout.c | 2 +- src/libostree/ostree-repo-pull.c | 14 +++++++------- src/ostree/ot-builtin-fsck.c | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libostree/ostree-bootconfig-parser.c b/src/libostree/ostree-bootconfig-parser.c index a2ac1072..8031334e 100644 --- a/src/libostree/ostree-bootconfig-parser.c +++ b/src/libostree/ostree-bootconfig-parser.c @@ -193,7 +193,7 @@ ostree_bootconfig_parser_write_at (OstreeBootconfigParser *self, else { write_key (self, buf, key, value); - g_hash_table_insert (written_overrides, (gpointer)key, (gpointer)key); + g_hash_table_add (written_overrides, (gpointer)key); } } diff --git a/src/libostree/ostree-repo-checkout.c b/src/libostree/ostree-repo-checkout.c index 9b640648..9a151646 100644 --- a/src/libostree/ostree-repo-checkout.c +++ b/src/libostree/ostree-repo-checkout.c @@ -554,7 +554,7 @@ checkout_one_file_at (OstreeRepo *repo, g_ascii_xdigit_value (checksum[1])); if (repo->updated_uncompressed_dirs == NULL) repo->updated_uncompressed_dirs = g_hash_table_new (NULL, NULL); - g_hash_table_insert (repo->updated_uncompressed_dirs, key, key); + g_hash_table_add (repo->updated_uncompressed_dirs, key); } g_mutex_unlock (&repo->cache_lock); diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 47da6846..b7197c38 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -522,7 +522,7 @@ scan_dirtree_object (OtPullData *pull_data, } else if (!file_is_stored && !g_hash_table_lookup (pull_data->requested_content, file_checksum)) { - g_hash_table_insert (pull_data->requested_content, file_checksum, file_checksum); + g_hash_table_add (pull_data->requested_content, file_checksum); enqueue_one_object_request (pull_data, file_checksum, OSTREE_OBJECT_TYPE_FILE, path, FALSE, FALSE); file_checksum = NULL; /* Transfer ownership */ } @@ -1292,7 +1292,7 @@ scan_one_metadata_object_c (OtPullData *pull_data, char *duped_checksum = g_strdup (tmp_checksum); gboolean do_fetch_detached; - g_hash_table_insert (pull_data->requested_metadata, duped_checksum, duped_checksum); + g_hash_table_add (pull_data->requested_metadata, duped_checksum); do_fetch_detached = (objtype == OSTREE_OBJECT_TYPE_COMMIT); enqueue_one_object_request (pull_data, tmp_checksum, objtype, path, do_fetch_detached, FALSE); @@ -1306,7 +1306,7 @@ scan_one_metadata_object_c (OtPullData *pull_data, pull_data->cancellable, error)) goto out; - g_hash_table_insert (pull_data->scanned_metadata, g_variant_ref (object), object); + g_hash_table_add (pull_data->scanned_metadata, g_variant_ref (object)); pull_data->n_scanned_metadata++; } else if (is_stored && objtype == OSTREE_OBJECT_TYPE_DIR_TREE) @@ -1315,7 +1315,7 @@ scan_one_metadata_object_c (OtPullData *pull_data, pull_data->cancellable, error)) goto out; - g_hash_table_insert (pull_data->scanned_metadata, g_variant_ref (object), object); + g_hash_table_add (pull_data->scanned_metadata, g_variant_ref (object)); pull_data->n_scanned_metadata++; } @@ -1536,7 +1536,7 @@ process_one_static_delta_fallback (OtPullData *pull_data, if (!g_hash_table_lookup (pull_data->requested_metadata, checksum)) { gboolean do_fetch_detached; - g_hash_table_insert (pull_data->requested_metadata, checksum, checksum); + g_hash_table_add (pull_data->requested_metadata, checksum); do_fetch_detached = (objtype == OSTREE_OBJECT_TYPE_COMMIT); enqueue_one_object_request (pull_data, checksum, objtype, NULL, do_fetch_detached, FALSE); @@ -1547,7 +1547,7 @@ process_one_static_delta_fallback (OtPullData *pull_data, { if (!g_hash_table_lookup (pull_data->requested_content, checksum)) { - g_hash_table_insert (pull_data->requested_content, checksum, checksum); + g_hash_table_add (pull_data->requested_content, checksum); enqueue_one_object_request (pull_data, checksum, OSTREE_OBJECT_TYPE_FILE, NULL, FALSE, FALSE); checksum = NULL; /* Transfer ownership */ } @@ -2836,7 +2836,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (ostree_validate_checksum_string (branch, NULL)) { char *key = g_strdup (branch); - g_hash_table_insert (commits_to_fetch, key, key); + g_hash_table_add (commits_to_fetch, key); } else { diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 9809ad29..090b44a0 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -311,7 +311,7 @@ ostree_builtin_fsck (int argc, char **argv, GCancellable *cancellable, GError ** if (commitstate & OSTREE_REPO_COMMIT_STATE_PARTIAL) n_partial++; else - g_hash_table_insert (commits, g_variant_ref (serialized_key), serialized_key); + g_hash_table_add (commits, g_variant_ref (serialized_key)); } } From daf01b27d4ae298d9f27e4b73b267917c1d5f5cb Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 6 Dec 2016 16:29:39 -0500 Subject: [PATCH 38/55] ci: Make all ci tests gating for Homu See the rhci docs. Closes: #617 Approved by: jlebon --- .redhat-ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.redhat-ci.yml b/.redhat-ci.yml index 6c170be1..4a3778ff 100644 --- a/.redhat-ci.yml +++ b/.redhat-ci.yml @@ -3,6 +3,8 @@ branches: - auto - try +required: true + container: image: projectatomic/ostree-tester @@ -30,6 +32,7 @@ artifacts: --- inherit: true +required: true context: Clang From a2d627352de9551e7b0dd93c2a7bc4dcb57b433b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 7 Dec 2016 09:55:10 -0500 Subject: [PATCH 39/55] build: Add more default errors Newer gcc has `-Wincompatible-pointer-types`, hooray! Add a few others that we pass today. Closes: #618 Approved by: jlebon --- configure.ac | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure.ac b/configure.ac index cb1cc980..ff27b0a0 100644 --- a/configure.ac +++ b/configure.ac @@ -29,6 +29,9 @@ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ -Werror=return-type \ -Werror=overflow \ -Werror=int-conversion \ + -Werror=parenthesis \ + -Werror=incompatible-pointer-types \ + -Werror=misleading-indentation \ -Werror=missing-include-dirs -Werror=aggregate-return \ -Werror=declaration-after-statement \ ]) From 099576ee4a8f5f23e17def823c44c5de31b14d5d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 6 Dec 2016 11:30:19 -0500 Subject: [PATCH 40/55] lib: Ensure we use _GNU_SOURCE in enum templates Due to the way glib-mkenums runs the preprocessor itself, it doesn't pick up the `AC_USE_SYSTEM_EXTENSIONS()` that we have in `configure.ac`. This blew up in an obscure way when I later wanted to `#include "libglnx.h"` in one of the headers, since it needs the `basename()` from `string.h` which is only available with `_GNU_SOURCE`. Closes: #616 Approved by: jlebon --- src/libostree/ostree-enumtypes.c.template | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libostree/ostree-enumtypes.c.template b/src/libostree/ostree-enumtypes.c.template index ab1b2aec..52da7de1 100644 --- a/src/libostree/ostree-enumtypes.c.template +++ b/src/libostree/ostree-enumtypes.c.template @@ -18,9 +18,15 @@ * Boston, MA 02111-1307, USA. */ +#ifndef _GNU_SOURCE +#define _GNU_SOURCE +#endif +#include + /*** END file-header ***/ /*** BEGIN file-production ***/ + /* enumerations from "@filename@" */ #include "@filename@" From 7519457382b91b7a0fc103d8ebe7e9db2c35b347 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 6 Dec 2016 11:34:05 -0500 Subject: [PATCH 41/55] fetcher: Define an abstraction over SoupURI This is preparatory work for a potential libcurl backend. Closes: #616 Approved by: jlebon --- src/libostree/ostree-fetcher.c | 104 +++++++++++++++++++++++---- src/libostree/ostree-fetcher.h | 37 ++++++++-- src/libostree/ostree-metalink.c | 26 +++---- src/libostree/ostree-metalink.h | 4 +- src/libostree/ostree-repo-pull.c | 118 ++++++++++++------------------- 5 files changed, 186 insertions(+), 103 deletions(-) diff --git a/src/libostree/ostree-fetcher.c b/src/libostree/ostree-fetcher.c index c9161d40..63942166 100644 --- a/src/libostree/ostree-fetcher.c +++ b/src/libostree/ostree-fetcher.c @@ -24,6 +24,10 @@ #include #include +#define LIBSOUP_USE_UNSTABLE_REQUEST_API +#include +#include +#include #include "libglnx.h" #include "ostree-fetcher.h" @@ -418,25 +422,20 @@ static void create_pending_soup_request (OstreeFetcherPendingURI *pending, GError **error) { - g_autofree char *uristr = NULL; - SoupURI *next_mirror = NULL; - SoupURI *uri = NULL; + OstreeFetcherURI *next_mirror = NULL; + g_autoptr(OstreeFetcherURI) uri = NULL; g_assert (pending->mirrorlist); g_assert (pending->mirrorlist_idx < pending->mirrorlist->len); - next_mirror = g_ptr_array_index (pending->mirrorlist, - pending->mirrorlist_idx); - uristr = g_build_filename (soup_uri_get_path (next_mirror), - pending->filename /* may be NULL */, NULL); - uri = soup_uri_copy (next_mirror); - soup_uri_set_path (uri, uristr); + next_mirror = g_ptr_array_index (pending->mirrorlist, pending->mirrorlist_idx); + if (pending->filename) + uri = _ostree_fetcher_uri_new_subpath (next_mirror, pending->filename); g_clear_object (&pending->request); pending->request = soup_session_request_uri (pending->thread_closure->session, - uri, error); - soup_uri_free (uri); + (SoupURI*)(uri ? uri : next_mirror), error); } static void @@ -1404,7 +1403,7 @@ _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, /* Helper for callers who just want to fetch single one-off URIs */ gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, - SoupURI *uri, + OstreeFetcherURI *uri, gboolean add_nul, gboolean allow_noent, GBytes **out_contents, @@ -1419,3 +1418,84 @@ _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, out_contents, max_size, cancellable, error); } + +void +_ostree_fetcher_uri_free (OstreeFetcherURI *uri) +{ + if (uri) + soup_uri_free ((SoupURI*)uri); +} + +OstreeFetcherURI * +_ostree_fetcher_uri_parse (const char *str, + GError **error) +{ + SoupURI *soupuri = soup_uri_new (str); + if (soupuri == NULL) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "Failed to parse uri: %s", str); + return NULL; + } + return (OstreeFetcherURI*)soupuri; +} + +static OstreeFetcherURI * +_ostree_fetcher_uri_new_path_internal (OstreeFetcherURI *uri, + gboolean extend, + const char *path) +{ + SoupURI *newuri = soup_uri_copy ((SoupURI*)uri); + if (path) + { + if (extend) + { + const char *origpath = soup_uri_get_path ((SoupURI*)uri); + g_autofree char *newpath = g_build_filename (origpath, path, NULL); + soup_uri_set_path (newuri, newpath); + } + else + { + soup_uri_set_path (newuri, path); + } + } + return (OstreeFetcherURI*)newuri; +} + +OstreeFetcherURI * +_ostree_fetcher_uri_new_path (OstreeFetcherURI *uri, + const char *path) +{ + return _ostree_fetcher_uri_new_path_internal (uri, FALSE, path); +} + +OstreeFetcherURI * +_ostree_fetcher_uri_new_subpath (OstreeFetcherURI *uri, + const char *subpath) +{ + return _ostree_fetcher_uri_new_path_internal (uri, TRUE, subpath); +} + +OstreeFetcherURI * +_ostree_fetcher_uri_clone (OstreeFetcherURI *uri) +{ + return _ostree_fetcher_uri_new_subpath (uri, NULL); +} + +char * +_ostree_fetcher_uri_get_scheme (OstreeFetcherURI *uri) +{ + return g_strdup (soup_uri_get_scheme ((SoupURI*)uri)); +} + +char * +_ostree_fetcher_uri_get_path (OstreeFetcherURI *uri) +{ + return g_strdup (soup_uri_get_path ((SoupURI*)uri)); +} + +char * +_ostree_fetcher_uri_to_string (OstreeFetcherURI *uri) +{ + return soup_uri_to_string ((SoupURI*)uri, FALSE); +} diff --git a/src/libostree/ostree-fetcher.h b/src/libostree/ostree-fetcher.h index 0bfba5b2..8e282e24 100644 --- a/src/libostree/ostree-fetcher.h +++ b/src/libostree/ostree-fetcher.h @@ -22,10 +22,7 @@ #ifndef __GI_SCANNER__ -#define LIBSOUP_USE_UNSTABLE_REQUEST_API -#include -#include -#include +#include "libglnx.h" G_BEGIN_DECLS @@ -39,6 +36,8 @@ G_BEGIN_DECLS /* Lower values have higher priority */ #define OSTREE_FETCHER_DEFAULT_PRIORITY 0 +typedef struct OstreeFetcherURI OstreeFetcherURI; + typedef struct OstreeFetcherClass OstreeFetcherClass; typedef struct OstreeFetcher OstreeFetcher; @@ -52,6 +51,34 @@ typedef enum { OSTREE_FETCHER_FLAGS_TLS_PERMISSIVE = (1 << 0) } OstreeFetcherConfigFlags; +void +_ostree_fetcher_uri_free (OstreeFetcherURI *uri); +G_DEFINE_AUTOPTR_CLEANUP_FUNC(OstreeFetcherURI, _ostree_fetcher_uri_free) + +OstreeFetcherURI * +_ostree_fetcher_uri_parse (const char *str, + GError **error); + +OstreeFetcherURI * +_ostree_fetcher_uri_clone (OstreeFetcherURI *uri); + +OstreeFetcherURI * +_ostree_fetcher_uri_new_path (OstreeFetcherURI *uri, + const char *subpath); + +OstreeFetcherURI * +_ostree_fetcher_uri_new_subpath (OstreeFetcherURI *uri, + const char *subpath); + +char * +_ostree_fetcher_uri_get_scheme (OstreeFetcherURI *uri); + +char * +_ostree_fetcher_uri_get_path (OstreeFetcherURI *uri); + +char * +_ostree_fetcher_uri_to_string (OstreeFetcherURI *uri); + GType _ostree_fetcher_get_type (void) G_GNUC_CONST; OstreeFetcher *_ostree_fetcher_new (int tmpdir_dfd, @@ -100,7 +127,7 @@ gboolean _ostree_fetcher_mirrored_request_to_membuf (OstreeFetcher *fetcher, GError **error); gboolean _ostree_fetcher_request_uri_to_membuf (OstreeFetcher *fetcher, - SoupURI *uri, + OstreeFetcherURI *uri, gboolean add_nul, gboolean allow_noent, GBytes **out_contents, diff --git a/src/libostree/ostree-metalink.c b/src/libostree/ostree-metalink.c index 981b30ed..ee52e51b 100644 --- a/src/libostree/ostree-metalink.c +++ b/src/libostree/ostree-metalink.c @@ -43,7 +43,7 @@ struct OstreeMetalink { GObject parent_instance; - SoupURI *uri; + OstreeFetcherURI *uri; OstreeFetcher *fetcher; char *requested_file; @@ -357,7 +357,7 @@ metalink_parser_text (GMarkupParseContext *context, case OSTREE_METALINK_STATE_URL: { g_autofree char *uri_text = g_strndup (text, text_len); - SoupURI *uri = soup_uri_new (uri_text); + OstreeFetcherURI *uri = _ostree_fetcher_uri_parse (uri_text, NULL); if (uri != NULL) g_ptr_array_add (self->urls, uri); } @@ -377,7 +377,7 @@ _ostree_metalink_finalize (GObject *object) g_object_unref (self->fetcher); g_free (self->requested_file); - soup_uri_free (self->uri); + _ostree_fetcher_uri_free (self->uri); G_OBJECT_CLASS (_ostree_metalink_parent_class)->finalize (object); } @@ -399,15 +399,15 @@ OstreeMetalink * _ostree_metalink_new (OstreeFetcher *fetcher, const char *requested_file, guint64 max_size, - SoupURI *uri) + OstreeFetcherURI *uri) { OstreeMetalink *self = (OstreeMetalink*)g_object_new (OSTREE_TYPE_METALINK, NULL); self->fetcher = g_object_ref (fetcher); self->requested_file = g_strdup (requested_file); self->max_size = max_size; - self->uri = soup_uri_copy (uri); - + self->uri = _ostree_fetcher_uri_clone (uri); + return self; } @@ -421,7 +421,7 @@ valid_hex_checksum (const char *s, gsize expected_len) static gboolean try_one_url (OstreeMetalinkRequest *self, - SoupURI *uri, + OstreeFetcherURI *uri, GBytes **out_data, GError **error) { @@ -486,12 +486,12 @@ try_one_url (OstreeMetalinkRequest *self, static gboolean try_metalink_targets (OstreeMetalinkRequest *self, - SoupURI **out_target_uri, + OstreeFetcherURI **out_target_uri, GBytes **out_data, GError **error) { gboolean ret = FALSE; - SoupURI *target_uri = NULL; + OstreeFetcherURI *target_uri = NULL; g_autoptr(GBytes) ret_data = NULL; if (!self->found_a_file_element) @@ -568,7 +568,7 @@ try_metalink_targets (OstreeMetalinkRequest *self, ret = TRUE; if (out_target_uri) - *out_target_uri = soup_uri_copy (target_uri); + *out_target_uri = _ostree_fetcher_uri_clone (target_uri); if (out_data) *out_data = g_steal_pointer (&ret_data); out: @@ -585,7 +585,7 @@ static const GMarkupParser metalink_parser = { typedef struct { - SoupURI **out_target_uri; + OstreeFetcherURI **out_target_uri; GBytes **out_data; gboolean success; GError **error; @@ -594,7 +594,7 @@ typedef struct gboolean _ostree_metalink_request_sync (OstreeMetalink *self, - SoupURI **out_target_uri, + OstreeFetcherURI **out_target_uri, GBytes **out_data, GCancellable *cancellable, GError **error) @@ -610,7 +610,7 @@ _ostree_metalink_request_sync (OstreeMetalink *self, g_main_context_push_thread_default (mainctx); request.metalink = g_object_ref (self); - request.urls = g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); + request.urls = g_ptr_array_new_with_free_func ((GDestroyNotify) _ostree_fetcher_uri_free); request.parser = g_markup_parse_context_new (&metalink_parser, G_MARKUP_PREFIX_ERROR_POSITION, &request, NULL); if (!_ostree_fetcher_request_uri_to_membuf (self->fetcher, diff --git a/src/libostree/ostree-metalink.h b/src/libostree/ostree-metalink.h index 9d09f007..55ed92e8 100644 --- a/src/libostree/ostree-metalink.h +++ b/src/libostree/ostree-metalink.h @@ -46,10 +46,10 @@ GType _ostree_metalink_get_type (void) G_GNUC_CONST; OstreeMetalink *_ostree_metalink_new (OstreeFetcher *fetcher, const char *requested_file, guint64 max_size, - SoupURI *uri); + OstreeFetcherURI *uri); gboolean _ostree_metalink_request_sync (OstreeMetalink *self, - SoupURI **out_target_uri, + OstreeFetcherURI **out_target_uri, GBytes **out_data, GCancellable *cancellable, GError **error); diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index b7197c38..0d33c048 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -353,7 +353,7 @@ fetch_mirrored_uri_contents_utf8_sync (OstreeFetcher *fetcher, static gboolean fetch_uri_contents_utf8_sync (OstreeFetcher *fetcher, - SoupURI *uri, + OstreeFetcherURI *uri, char **out_contents, GCancellable *cancellable, GError **error) @@ -2055,17 +2055,13 @@ fetch_mirrorlist (OstreeFetcher *fetcher, gboolean ret = FALSE; g_auto(GStrv) lines = NULL; g_autofree char *contents = NULL; - SoupURI *mirrorlist = NULL; + g_autoptr(OstreeFetcherURI) mirrorlist = NULL; g_autoptr(GPtrArray) ret_mirrorlist = - g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); + g_ptr_array_new_with_free_func ((GDestroyNotify) _ostree_fetcher_uri_free); - mirrorlist = soup_uri_new (mirrorlist_url); - if (mirrorlist == NULL) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Failed to parse mirrorlist URL '%s'", mirrorlist_url); - goto out; - } + mirrorlist = _ostree_fetcher_uri_parse (mirrorlist_url, error); + if (!mirrorlist) + goto out; if (!fetch_uri_contents_utf8_sync (fetcher, mirrorlist, &contents, cancellable, error)) @@ -2083,27 +2079,28 @@ fetch_mirrorlist (OstreeFetcher *fetcher, for (char **iter = lines; iter && *iter; iter++) { const char *mirror_uri_str = *iter; - SoupURI *mirror_uri = NULL; + g_autoptr(OstreeFetcherURI) mirror_uri = NULL; + g_autofree char *scheme = NULL; /* let's be nice and support empty lines and comments */ if (*mirror_uri_str == '\0' || *mirror_uri_str == '#') continue; - mirror_uri = soup_uri_new (mirror_uri_str); - if (mirror_uri == NULL) + mirror_uri = _ostree_fetcher_uri_parse (mirror_uri_str, NULL); + if (!mirror_uri) { g_debug ("Can't parse mirrorlist line '%s'", mirror_uri_str); continue; } - else if ((strcmp (soup_uri_get_scheme (mirror_uri), "http") != 0) && - (strcmp (soup_uri_get_scheme (mirror_uri), "https") != 0)) + + scheme = _ostree_fetcher_uri_get_scheme (mirror_uri); + if (!(g_str_equal (scheme, "http") || (g_str_equal (scheme, "https")))) { /* let's not support mirrorlists that contain non-http based URIs for * now (e.g. local URIs) -- we need to think about if and how we want * to support this since we set up things differently depending on * whether we're pulling locally or not */ g_debug ("Ignoring non-http/s mirrorlist entry '%s'", mirror_uri_str); - soup_uri_free (mirror_uri); continue; } @@ -2114,9 +2111,7 @@ fetch_mirrorlist (OstreeFetcher *fetcher, if (ret_mirrorlist->len == 0) { GError *local_error = NULL; - g_autofree char *config_uri_str = g_build_filename (mirror_uri_str, - "config", NULL); - SoupURI *config_uri = soup_uri_new (config_uri_str); + g_autoptr(OstreeFetcherURI) config_uri = _ostree_fetcher_uri_new_subpath (mirror_uri, "config"); if (fetch_uri_contents_utf8_sync (fetcher, config_uri, NULL, cancellable, &local_error)) @@ -2127,16 +2122,11 @@ fetch_mirrorlist (OstreeFetcher *fetcher, mirror_uri_str, local_error->message); g_clear_error (&local_error); } - - soup_uri_free (config_uri); } else { g_ptr_array_add (ret_mirrorlist, g_steal_pointer (&mirror_uri)); } - - if (mirror_uri != NULL) - soup_uri_free (mirror_uri); } if (ret_mirrorlist->len == 0) @@ -2151,8 +2141,6 @@ fetch_mirrorlist (OstreeFetcher *fetcher, ret = TRUE; out: - if (mirrorlist != NULL) - soup_uri_free (mirrorlist); return ret; } @@ -2201,18 +2189,14 @@ repo_remote_fetch_summary (OstreeRepo *self, } else { - SoupURI *uri = soup_uri_new (url_string); + g_autoptr(OstreeFetcherURI) uri = _ostree_fetcher_uri_parse (url_string, error); - if (uri == NULL) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Failed to parse url '%s'", url_string); - goto out; - } + if (!uri) + goto out; mirrorlist = - g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); - g_ptr_array_add (mirrorlist, uri /* transfer ownership */ ); + g_ptr_array_new_with_free_func ((GDestroyNotify) _ostree_fetcher_uri_free); + g_ptr_array_add (mirrorlist, g_steal_pointer (&uri)); } } @@ -2522,36 +2506,27 @@ ostree_repo_pull_with_options (OstreeRepo *self, } else { - SoupURI *baseuri = soup_uri_new (baseurl); + g_autoptr(OstreeFetcherURI) baseuri = _ostree_fetcher_uri_parse (baseurl, error); - if (baseuri == NULL) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Failed to parse url '%s'", baseurl); - goto out; - } + if (!baseuri) + goto out; pull_data->meta_mirrorlist = - g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); - g_ptr_array_add (pull_data->meta_mirrorlist, baseuri /* transfer */); + g_ptr_array_new_with_free_func ((GDestroyNotify) _ostree_fetcher_uri_free); + g_ptr_array_add (pull_data->meta_mirrorlist, g_steal_pointer (&baseuri)); } } else { g_autoptr(GBytes) summary_bytes = NULL; - SoupURI *metalink_uri = soup_uri_new (metalink_url_str); - SoupURI *target_uri = NULL; - + g_autoptr(OstreeFetcherURI) metalink_uri = _ostree_fetcher_uri_parse (metalink_url_str, error); + g_autoptr(OstreeFetcherURI) target_uri = NULL; + if (!metalink_uri) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Invalid metalink URL: %s", metalink_url_str); - goto out; - } - + goto out; + metalink = _ostree_metalink_new (pull_data->fetcher, "summary", OSTREE_MAX_METADATA_SIZE, metalink_uri); - soup_uri_free (metalink_uri); if (! _ostree_metalink_request_sync (metalink, &target_uri, @@ -2564,12 +2539,12 @@ ostree_repo_pull_with_options (OstreeRepo *self, * mirrors here since we use it as such anyway (rather than the "usual" * use case of metalink, which is only for a single target filename) */ { - /* reuse target_uri and take ownership */ - g_autofree char *repo_base = g_path_get_dirname (soup_uri_get_path (target_uri)); - soup_uri_set_path (target_uri, repo_base); + g_autofree char *path = _ostree_fetcher_uri_get_path (target_uri); + g_autofree char *basepath = g_path_get_dirname (path); + g_autoptr(OstreeFetcherURI) new_target_uri = _ostree_fetcher_uri_new_path (target_uri, basepath); pull_data->meta_mirrorlist = - g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); - g_ptr_array_add (pull_data->meta_mirrorlist, target_uri); + g_ptr_array_new_with_free_func ((GDestroyNotify) _ostree_fetcher_uri_free); + g_ptr_array_add (pull_data->meta_mirrorlist, g_steal_pointer (&new_target_uri)); } pull_data->summary = g_variant_new_from_bytes (OSTREE_SUMMARY_GVARIANT_FORMAT, @@ -2603,19 +2578,15 @@ ostree_repo_pull_with_options (OstreeRepo *self, } else { - SoupURI *contenturi = soup_uri_new (contenturl); + g_autoptr(OstreeFetcherURI) contenturi = _ostree_fetcher_uri_parse (contenturl, error); - if (contenturi == NULL) - { - g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, - "Failed to parse contenturl '%s'", contenturl); - goto out; - } + if (!contenturi) + goto out; pull_data->content_mirrorlist = - g_ptr_array_new_with_free_func ((GDestroyNotify) soup_uri_free); + g_ptr_array_new_with_free_func ((GDestroyNotify) _ostree_fetcher_uri_free); g_ptr_array_add (pull_data->content_mirrorlist, - contenturi /* transfer */); + g_steal_pointer (&contenturi)); } } } @@ -2625,12 +2596,16 @@ ostree_repo_pull_with_options (OstreeRepo *self, &configured_branches, error)) goto out; + /* TODO reindent later */ + { OstreeFetcherURI *first_uri = pull_data->meta_mirrorlist->pdata[0]; + g_autofree char *first_scheme = _ostree_fetcher_uri_get_scheme (first_uri); + /* NB: we don't support local mirrors in mirrorlists, so if this passes, it * means that we're not using mirrorlists (see also fetch_mirrorlist()) */ - if (strcmp (soup_uri_get_scheme (pull_data->meta_mirrorlist->pdata[0]), "file") == 0) + if (g_str_equal (first_scheme, "file")) { - g_autoptr(GFile) remote_repo_path = - g_file_new_for_path (soup_uri_get_path (pull_data->meta_mirrorlist->pdata[0])); + g_autofree char *path = _ostree_fetcher_uri_get_path (first_uri); + g_autoptr(GFile) remote_repo_path = g_file_new_for_path (path); pull_data->remote_repo_local = ostree_repo_new (remote_repo_path); if (!ostree_repo_open (pull_data->remote_repo_local, cancellable, error)) goto out; @@ -2659,6 +2634,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, goto out; } } + } /* For local pulls, default to disabling static deltas so that the * exact object files are copied. From 7f2830cb4e234c89a5aeaf600717d291ec09a6c0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 7 Dec 2016 21:16:52 -0500 Subject: [PATCH 42/55] build: Make libsoup optional again The "remote cookies" code broke this. While I'm not sure anyone is actually using ostree-without-http, it isn't too hard to keep the build time conditional going. Further, this work is preparatory for libcurl porting. Closes: #621 Approved by: jlebon --- Makefile-ostree.am | 11 ++++++++--- Makefile-tests.am | 5 ++++- src/ostree/ot-builtin-remote.c | 8 +++++--- src/ostree/ot-remote-builtins.h | 6 ++++-- 4 files changed, 21 insertions(+), 9 deletions(-) diff --git a/Makefile-ostree.am b/Makefile-ostree.am index 9f2119dc..e6b1eabe 100644 --- a/Makefile-ostree.am +++ b/Makefile-ostree.am @@ -80,17 +80,22 @@ ostree_SOURCES += \ ostree_SOURCES += \ src/ostree/ot-remote-builtins.h \ src/ostree/ot-remote-builtin-add.c \ - src/ostree/ot-remote-builtin-add-cookie.c \ src/ostree/ot-remote-builtin-delete.c \ - src/ostree/ot-remote-builtin-delete-cookie.c \ src/ostree/ot-remote-builtin-gpg-import.c \ src/ostree/ot-remote-builtin-list.c \ - src/ostree/ot-remote-builtin-list-cookies.c \ src/ostree/ot-remote-builtin-show-url.c \ src/ostree/ot-remote-builtin-refs.c \ src/ostree/ot-remote-builtin-summary.c \ $(NULL) +if USE_LIBSOUP +ostree_SOURCES += \ + src/ostree/ot-remote-builtin-add-cookie.c \ + src/ostree/ot-remote-builtin-delete-cookie.c \ + src/ostree/ot-remote-builtin-list-cookies.c \ + $(NULL) +endif + src/ostree/parse-datetime.c: src/ostree/parse-datetime.y Makefile $(AM_V_GEN) $(YACC) $< -o $@ diff --git a/Makefile-tests.am b/Makefile-tests.am index d4685237..5782fcbd 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -44,7 +44,6 @@ dist_test_scripts = \ tests/test-pull-subpath.sh \ tests/test-archivez.sh \ tests/test-remote-add.sh \ - tests/test-remote-cookies.sh \ tests/test-remote-headers.sh \ tests/test-remote-gpg-import.sh \ tests/test-commit-sign.sh \ @@ -101,6 +100,10 @@ else EXTRA_DIST += tests/test-rofiles-fuse.sh endif +if USE_LIBSOUP +dist_test_scripts += tests/test-remote-cookies.sh +endif + # This one uses corrupt-repo-ref.js js_tests = tests/test-corruption.sh if BUILDOPT_GJS diff --git a/src/ostree/ot-builtin-remote.c b/src/ostree/ot-builtin-remote.c index 31924eb1..57c3ae09 100644 --- a/src/ostree/ot-builtin-remote.c +++ b/src/ostree/ot-builtin-remote.c @@ -33,13 +33,15 @@ typedef struct { static OstreeRemoteCommand remote_subcommands[] = { { "add", ot_remote_builtin_add }, - { "add-cookie", ot_remote_builtin_add_cookie }, { "delete", ot_remote_builtin_delete }, - { "delete-cookie", ot_remote_builtin_delete_cookie }, { "show-url", ot_remote_builtin_show_url }, { "list", ot_remote_builtin_list }, - { "list-cookies", ot_remote_builtin_list_cookies }, { "gpg-import", ot_remote_builtin_gpg_import }, +#ifdef HAVE_LIBSOUP + { "add-cookie", ot_remote_builtin_add_cookie }, + { "delete-cookie", ot_remote_builtin_delete_cookie }, + { "list-cookies", ot_remote_builtin_list_cookies }, +#endif { "refs", ot_remote_builtin_refs }, { "summary", ot_remote_builtin_summary }, { NULL, NULL } diff --git a/src/ostree/ot-remote-builtins.h b/src/ostree/ot-remote-builtins.h index 289e2e0d..aa2a7b6c 100644 --- a/src/ostree/ot-remote-builtins.h +++ b/src/ostree/ot-remote-builtins.h @@ -25,12 +25,14 @@ G_BEGIN_DECLS gboolean ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError **error); -gboolean ot_remote_builtin_add_cookie (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_delete (int argc, char **argv, GCancellable *cancellable, GError **error); -gboolean ot_remote_builtin_delete_cookie (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_gpg_import (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_list (int argc, char **argv, GCancellable *cancellable, GError **error); +#ifdef HAVE_LIBSOUP +gboolean ot_remote_builtin_add_cookie (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_list_cookies (int argc, char **argv, GCancellable *cancellable, GError **error); +gboolean ot_remote_builtin_delete_cookie (int argc, char **argv, GCancellable *cancellable, GError **error); +#endif gboolean ot_remote_builtin_show_url (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_refs (int argc, char **argv, GCancellable *cancellable, GError **error); gboolean ot_remote_builtin_summary (int argc, char **argv, GCancellable *cancellable, GError **error); From 58e318d678c8bd72b536a187fe2523fc114515d9 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Dec 2016 10:01:35 -0500 Subject: [PATCH 43/55] [ASAN] sysroot: Squash a leak in lockfile acquisition I installed `parallel` in my dev container, which got me the sysroot locking tests, which caught this leak when built with ASAN. Closes: #623 Approved by: jlebon --- src/libostree/ostree-sysroot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 70ce1567..f50e34bd 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -1323,7 +1323,7 @@ ostree_sysroot_try_lock (OstreeSysroot *self, GError **error) { gboolean ret = FALSE; - GError *local_error = NULL; + g_autoptr(GError) local_error = NULL; if (!ensure_sysroot_fd (self, error)) goto out; @@ -1338,7 +1338,7 @@ ostree_sysroot_try_lock (OstreeSysroot *self, } else { - g_propagate_error (error, local_error); + g_propagate_error (error, g_steal_pointer (&local_error)); goto out; } } From 5155662b57fe5943bba014c3035d87229f55ce54 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 7 Dec 2016 21:00:21 -0500 Subject: [PATCH 44/55] build: Always do enum scanning now Since we stopped including the libsoup headers in `ostree-fetcher.h`, we can now unconditionally do enum scanning, and drop a build time conditional. Prep for libcurl porting. Closes: #620 Approved by: jlebon --- Makefile-libostree.am | 6 ------ 1 file changed, 6 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index d6ae6023..a7d7b11b 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -34,10 +34,7 @@ libostreeincludedir = $(includedir)/ostree-1 libostreeinclude_HEADERS = $(libostree_public_headers) ENUM_TYPES = $(NULL) - -if USE_LIBSOUP ENUM_TYPES += $(srcdir)/src/libostree/ostree-fetcher.h -endif src/libostree/ostree-enumtypes.h: src/libostree/ostree-enumtypes.h.template $(ENUM_TYPES) $(AM_V_GEN) $(GLIB_MKENUMS) \ @@ -50,14 +47,11 @@ src/libostree/ostree-enumtypes.c: src/libostree/ostree-enumtypes.c.template $(EN --fhead "#include \"ostree-enumtypes.h\"" \ $(ENUM_TYPES) > $@.tmp && mv $@.tmp $@ -if USE_LIBSOUP ENUM_GENERATED = \ src/libostree/ostree-enumtypes.h \ src/libostree/ostree-enumtypes.c \ $(NULL) - BUILT_SOURCES += $(ENUM_GENERATED) -endif CLEANFILES += $(BUILT_SOURCES) From c19fae0282f52581665c62c74ab808b80382f301 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Dec 2016 10:32:09 -0500 Subject: [PATCH 45/55] tree-wide: Switch to autoptr for GOptionContext We were leaking in a few places that I noticed in an ASAN run. Also, this was one of the last non-autoptr cleanup sections we have in `out:` cleanup sections, making us a lot closer to a potential full-tree rewrite to `return FALSE`. Closes: #624 Approved by: jlebon --- src/ostree/ot-admin-builtin-cleanup.c | 4 +--- src/ostree/ot-admin-builtin-deploy.c | 4 +--- src/ostree/ot-admin-builtin-diff.c | 4 +--- src/ostree/ot-admin-builtin-init-fs.c | 4 +--- src/ostree/ot-admin-builtin-instutil.c | 4 +--- src/ostree/ot-admin-builtin-os-init.c | 4 +--- src/ostree/ot-admin-builtin-set-origin.c | 4 +--- src/ostree/ot-admin-builtin-status.c | 4 +--- src/ostree/ot-admin-builtin-switch.c | 4 +--- src/ostree/ot-admin-builtin-undeploy.c | 4 +--- src/ostree/ot-admin-builtin-unlock.c | 4 +--- src/ostree/ot-admin-builtin-upgrade.c | 4 +--- ...ot-admin-instutil-builtin-grub2-generate.c | 4 +--- ...-instutil-builtin-selinux-ensure-labeled.c | 4 +--- .../ot-admin-instutil-builtin-set-kargs.c | 4 +--- src/ostree/ot-builtin-admin.c | 4 +--- src/ostree/ot-builtin-cat.c | 4 +--- src/ostree/ot-builtin-checkout.c | 4 +--- src/ostree/ot-builtin-checksum.c | 4 +--- src/ostree/ot-builtin-commit.c | 4 +--- src/ostree/ot-builtin-config.c | 4 +--- src/ostree/ot-builtin-diff.c | 4 +--- src/ostree/ot-builtin-export.c | 4 +--- src/ostree/ot-builtin-fsck.c | 4 +--- src/ostree/ot-builtin-gpg-sign.c | 5 +---- src/ostree/ot-builtin-init.c | 4 +--- src/ostree/ot-builtin-log.c | 4 +--- src/ostree/ot-builtin-ls.c | 4 +--- src/ostree/ot-builtin-prune.c | 4 +--- src/ostree/ot-builtin-pull-local.c | 4 +--- src/ostree/ot-builtin-pull.c | 4 +--- src/ostree/ot-builtin-refs.c | 4 +--- src/ostree/ot-builtin-remote.c | 4 +--- src/ostree/ot-builtin-reset.c | 4 +--- src/ostree/ot-builtin-rev-parse.c | 4 +--- src/ostree/ot-builtin-show.c | 4 +--- src/ostree/ot-builtin-static-delta.c | 20 +++++-------------- src/ostree/ot-builtin-summary.c | 4 +--- src/ostree/ot-main.c | 8 ++------ src/ostree/ot-remote-builtin-add-cookie.c | 4 +--- src/ostree/ot-remote-builtin-add.c | 4 +--- src/ostree/ot-remote-builtin-delete-cookie.c | 4 +--- src/ostree/ot-remote-builtin-delete.c | 4 +--- src/ostree/ot-remote-builtin-gpg-import.c | 4 +--- src/ostree/ot-remote-builtin-list-cookies.c | 2 +- src/ostree/ot-remote-builtin-list.c | 4 +--- src/ostree/ot-remote-builtin-refs.c | 5 +---- src/ostree/ot-remote-builtin-summary.c | 5 +---- 48 files changed, 53 insertions(+), 160 deletions(-) diff --git a/src/ostree/ot-admin-builtin-cleanup.c b/src/ostree/ot-admin-builtin-cleanup.c index 7a76c14b..a6f74e38 100644 --- a/src/ostree/ot-admin-builtin-cleanup.c +++ b/src/ostree/ot-admin-builtin-cleanup.c @@ -36,7 +36,7 @@ static GOptionEntry options[] = { gboolean ot_admin_builtin_cleanup (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; gboolean ret = FALSE; @@ -55,7 +55,5 @@ ot_admin_builtin_cleanup (int argc, char **argv, GCancellable *cancellable, GErr ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-deploy.c b/src/ostree/ot-admin-builtin-deploy.c index 3039cf0d..e59bec8e 100644 --- a/src/ostree/ot-admin-builtin-deploy.c +++ b/src/ostree/ot-admin-builtin-deploy.c @@ -54,7 +54,7 @@ ot_admin_builtin_deploy (int argc, char **argv, GCancellable *cancellable, GErro { gboolean ret = FALSE; const char *refspec; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; GKeyFile *origin = NULL; glnx_unref_object OstreeRepo *repo = NULL; @@ -173,7 +173,5 @@ ot_admin_builtin_deploy (int argc, char **argv, GCancellable *cancellable, GErro out: if (origin) g_key_file_unref (origin); - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-diff.c b/src/ostree/ot-admin-builtin-diff.c index 85a820ac..2615f24c 100644 --- a/src/ostree/ot-admin-builtin-diff.c +++ b/src/ostree/ot-admin-builtin-diff.c @@ -39,7 +39,7 @@ static GOptionEntry options[] = { gboolean ot_admin_builtin_diff (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; gboolean ret = FALSE; glnx_unref_object OstreeDeployment *deployment = NULL; @@ -95,7 +95,5 @@ ot_admin_builtin_diff (int argc, char **argv, GCancellable *cancellable, GError ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-init-fs.c b/src/ostree/ot-admin-builtin-init-fs.c index 1f122553..9c08630d 100644 --- a/src/ostree/ot-admin-builtin-init-fs.c +++ b/src/ostree/ot-admin-builtin-init-fs.c @@ -36,7 +36,7 @@ static GOptionEntry options[] = { gboolean ot_admin_builtin_init_fs (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; gboolean ret = FALSE; glnx_fd_close int root_dfd = -1; @@ -88,7 +88,5 @@ ot_admin_builtin_init_fs (int argc, char **argv, GCancellable *cancellable, GErr ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-instutil.c b/src/ostree/ot-admin-builtin-instutil.c index 88894a6a..f27316a3 100644 --- a/src/ostree/ot-admin-builtin-instutil.c +++ b/src/ostree/ot-admin-builtin-instutil.c @@ -109,7 +109,7 @@ ot_admin_builtin_instutil (int argc, char **argv, GCancellable *cancellable, GEr if (!subcommand->name) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; g_autofree char *help; context = ostree_admin_instutil_option_context_new_with_commands (); @@ -134,8 +134,6 @@ ot_admin_builtin_instutil (int argc, char **argv, GCancellable *cancellable, GEr help = g_option_context_get_help (context, FALSE, NULL); g_printerr ("%s", help); - g_option_context_free (context); - goto out; } diff --git a/src/ostree/ot-admin-builtin-os-init.c b/src/ostree/ot-admin-builtin-os-init.c index fbb04949..cb95bebd 100644 --- a/src/ostree/ot-admin-builtin-os-init.c +++ b/src/ostree/ot-admin-builtin-os-init.c @@ -36,7 +36,7 @@ static GOptionEntry options[] = { gboolean ot_admin_builtin_os_init (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; gboolean ret = FALSE; const char *osname = NULL; @@ -66,7 +66,5 @@ ot_admin_builtin_os_init (int argc, char **argv, GCancellable *cancellable, GErr ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-set-origin.c b/src/ostree/ot-admin-builtin-set-origin.c index 2b6866cc..a814d9d5 100644 --- a/src/ostree/ot-admin-builtin-set-origin.c +++ b/src/ostree/ot-admin-builtin-set-origin.c @@ -44,7 +44,7 @@ gboolean ot_admin_builtin_set_origin (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; const char *remotename = NULL; const char *url = NULL; const char *branch = NULL; @@ -144,7 +144,5 @@ ot_admin_builtin_set_origin (int argc, char **argv, GCancellable *cancellable, G ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-status.c b/src/ostree/ot-admin-builtin-status.c index 2550bcea..940e5df0 100644 --- a/src/ostree/ot-admin-builtin-status.c +++ b/src/ostree/ot-admin-builtin-status.c @@ -83,7 +83,7 @@ out: gboolean ot_admin_builtin_status (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; gboolean ret = FALSE; glnx_unref_object OstreeRepo *repo = NULL; @@ -193,7 +193,5 @@ ot_admin_builtin_status (int argc, char **argv, GCancellable *cancellable, GErro ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-switch.c b/src/ostree/ot-admin-builtin-switch.c index 485e6cd4..877cbe96 100644 --- a/src/ostree/ot-admin-builtin-switch.c +++ b/src/ostree/ot-admin-builtin-switch.c @@ -43,7 +43,7 @@ gboolean ot_admin_builtin_switch (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; const char *new_provided_refspec = NULL; glnx_unref_object OstreeRepo *repo = NULL; @@ -162,7 +162,5 @@ ot_admin_builtin_switch (int argc, char **argv, GCancellable *cancellable, GErro out: if (new_origin) g_key_file_unref (new_origin); - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-undeploy.c b/src/ostree/ot-admin-builtin-undeploy.c index dd41950e..cc86ee43 100644 --- a/src/ostree/ot-admin-builtin-undeploy.c +++ b/src/ostree/ot-admin-builtin-undeploy.c @@ -36,7 +36,7 @@ gboolean ot_admin_builtin_undeploy (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; const char *deploy_index_str; int deploy_index; @@ -91,7 +91,5 @@ ot_admin_builtin_undeploy (int argc, char **argv, GCancellable *cancellable, GEr ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-unlock.c b/src/ostree/ot-admin-builtin-unlock.c index a1789377..0f22d0a6 100644 --- a/src/ostree/ot-admin-builtin-unlock.c +++ b/src/ostree/ot-admin-builtin-unlock.c @@ -42,7 +42,7 @@ gboolean ot_admin_builtin_unlock (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; OstreeDeployment *booted_deployment = NULL; OstreeDeploymentUnlockedState target_state; @@ -95,7 +95,5 @@ ot_admin_builtin_unlock (int argc, char **argv, GCancellable *cancellable, GErro ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-builtin-upgrade.c b/src/ostree/ot-admin-builtin-upgrade.c index 2ad74fc6..394b339a 100644 --- a/src/ostree/ot-admin-builtin-upgrade.c +++ b/src/ostree/ot-admin-builtin-upgrade.c @@ -49,7 +49,7 @@ gboolean ot_admin_builtin_upgrade (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; glnx_unref_object OstreeSysrootUpgrader *upgrader = NULL; g_autoptr(GKeyFile) origin = NULL; @@ -140,7 +140,5 @@ ot_admin_builtin_upgrade (int argc, char **argv, GCancellable *cancellable, GErr ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-instutil-builtin-grub2-generate.c b/src/ostree/ot-admin-instutil-builtin-grub2-generate.c index 7d837ff4..4b8b581b 100644 --- a/src/ostree/ot-admin-instutil-builtin-grub2-generate.c +++ b/src/ostree/ot-admin-instutil-builtin-grub2-generate.c @@ -38,7 +38,7 @@ ot_admin_instutil_builtin_grub2_generate (int argc, char **argv, GCancellable *c { gboolean ret = FALSE; guint bootversion; - GOptionContext *context = NULL; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; context = g_option_context_new ("[BOOTVERSION] - generate GRUB2 configuration from given BLS entries"); @@ -76,7 +76,5 @@ ot_admin_instutil_builtin_grub2_generate (int argc, char **argv, GCancellable *c ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-instutil-builtin-selinux-ensure-labeled.c b/src/ostree/ot-admin-instutil-builtin-selinux-ensure-labeled.c index 692a2623..424c7645 100644 --- a/src/ostree/ot-admin-instutil-builtin-selinux-ensure-labeled.c +++ b/src/ostree/ot-admin-instutil-builtin-selinux-ensure-labeled.c @@ -188,7 +188,7 @@ ot_admin_instutil_builtin_selinux_ensure_labeled (int argc, char **argv, GCancel glnx_unref_object OstreeSePolicy *sepolicy = NULL; g_autoptr(GPtrArray) deployments = NULL; OstreeDeployment *first_deployment; - GOptionContext *context = NULL; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; g_autoptr(GFile) deployment_path = NULL; @@ -241,7 +241,5 @@ ot_admin_instutil_builtin_selinux_ensure_labeled (int argc, char **argv, GCancel ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-admin-instutil-builtin-set-kargs.c b/src/ostree/ot-admin-instutil-builtin-set-kargs.c index c51fc4d2..ad792a3a 100644 --- a/src/ostree/ot-admin-instutil-builtin-set-kargs.c +++ b/src/ostree/ot-admin-instutil-builtin-set-kargs.c @@ -50,7 +50,7 @@ ot_admin_instutil_builtin_set_kargs (int argc, char **argv, GCancellable *cancel guint i; g_autoptr(GPtrArray) deployments = NULL; OstreeDeployment *first_deployment = NULL; - GOptionContext *context = NULL; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeSysroot *sysroot = NULL; __attribute__((cleanup(_ostree_kernel_args_cleanup))) OstreeKernelArgs *kargs = NULL; @@ -115,7 +115,5 @@ ot_admin_instutil_builtin_set_kargs (int argc, char **argv, GCancellable *cancel ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-admin.c b/src/ostree/ot-builtin-admin.c index 7e7b04b5..4760e532 100644 --- a/src/ostree/ot-builtin-admin.c +++ b/src/ostree/ot-builtin-admin.c @@ -124,7 +124,7 @@ ostree_builtin_admin (int argc, char **argv, GCancellable *cancellable, GError * if (!subcommand->name) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; g_autofree char *help; context = ostree_admin_option_context_new_with_commands (); @@ -149,8 +149,6 @@ ostree_builtin_admin (int argc, char **argv, GCancellable *cancellable, GError * help = g_option_context_get_help (context, FALSE, NULL); g_printerr ("%s", help); - g_option_context_free (context); - goto out; } diff --git a/src/ostree/ot-builtin-cat.c b/src/ostree/ot-builtin-cat.c index e258f3f7..a784afe1 100644 --- a/src/ostree/ot-builtin-cat.c +++ b/src/ostree/ot-builtin-cat.c @@ -61,7 +61,7 @@ cat_one_file (GFile *f, gboolean ostree_builtin_cat (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; int i; @@ -98,7 +98,5 @@ ostree_builtin_cat (int argc, char **argv, GCancellable *cancellable, GError **e ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-checkout.c b/src/ostree/ot-builtin-checkout.c index 21e568b2..95172f8b 100644 --- a/src/ostree/ot-builtin-checkout.c +++ b/src/ostree/ot-builtin-checkout.c @@ -230,7 +230,7 @@ process_many_checkouts (OstreeRepo *repo, gboolean ostree_builtin_checkout (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; const char *commit; @@ -281,7 +281,5 @@ ostree_builtin_checkout (int argc, char **argv, GCancellable *cancellable, GErro ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-checksum.c b/src/ostree/ot-builtin-checksum.c index 09697c88..540b02a1 100644 --- a/src/ostree/ot-builtin-checksum.c +++ b/src/ostree/ot-builtin-checksum.c @@ -58,7 +58,7 @@ on_checksum_received (GObject *obj, gboolean ostree_builtin_checksum (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; gboolean ret = FALSE; g_autoptr(GFile) f = NULL; AsyncChecksumData data = { 0, }; @@ -87,7 +87,5 @@ ostree_builtin_checksum (int argc, char **argv, GCancellable *cancellable, GErro out: if (data.loop) g_main_loop_unref (data.loop); - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index fa829817..d3b634f9 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -331,7 +331,7 @@ parse_keyvalue_strings (char **strings, gboolean ostree_builtin_commit (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; gboolean skip_commit = FALSE; @@ -672,8 +672,6 @@ ostree_builtin_commit (int argc, char **argv, GCancellable *cancellable, GError out: if (repo) ostree_repo_abort_transaction (repo, cancellable, NULL); - if (context) - g_option_context_free (context); if (modifier) ostree_repo_commit_modifier_unref (modifier); return ret; diff --git a/src/ostree/ot-builtin-config.c b/src/ostree/ot-builtin-config.c index 75ccbeb0..0d8f7b77 100644 --- a/src/ostree/ot-builtin-config.c +++ b/src/ostree/ot-builtin-config.c @@ -55,7 +55,7 @@ split_key_string (const char *k, gboolean ostree_builtin_config (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context = NULL; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; const char *op; @@ -133,7 +133,5 @@ ostree_builtin_config (int argc, char **argv, GCancellable *cancellable, GError out: if (config) g_key_file_free (config); - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-diff.c b/src/ostree/ot-builtin-diff.c index a23ed83f..963a8b7c 100644 --- a/src/ostree/ot-builtin-diff.c +++ b/src/ostree/ot-builtin-diff.c @@ -121,7 +121,7 @@ gboolean ostree_builtin_diff (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *src; const char *target; @@ -224,7 +224,5 @@ ostree_builtin_diff (int argc, char **argv, GCancellable *cancellable, GError ** ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-export.c b/src/ostree/ot-builtin-export.c index b0bb40fa..f3cdfbe0 100644 --- a/src/ostree/ot-builtin-export.c +++ b/src/ostree/ot-builtin-export.c @@ -60,7 +60,7 @@ propagate_libarchive_error (GError **error, gboolean ostree_builtin_export (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; const char *rev; @@ -155,7 +155,5 @@ ostree_builtin_export (int argc, char **argv, GCancellable *cancellable, GError ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-fsck.c b/src/ostree/ot-builtin-fsck.c index 090b44a0..2f154cff 100644 --- a/src/ostree/ot-builtin-fsck.c +++ b/src/ostree/ot-builtin-fsck.c @@ -240,7 +240,7 @@ gboolean ostree_builtin_fsck (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; GHashTableIter hash_iter; gpointer key, value; @@ -355,7 +355,5 @@ ostree_builtin_fsck (int argc, char **argv, GCancellable *cancellable, GError ** ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-gpg-sign.c b/src/ostree/ot-builtin-gpg-sign.c index 221e2004..c19ec682 100644 --- a/src/ostree/ot-builtin-gpg-sign.c +++ b/src/ostree/ot-builtin-gpg-sign.c @@ -197,7 +197,7 @@ out: gboolean ostree_builtin_gpg_sign (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; g_autofree char *resolved_commit = NULL; const char *commit; @@ -254,8 +254,5 @@ ostree_builtin_gpg_sign (int argc, char **argv, GCancellable *cancellable, GErro ret = TRUE; out: - if (context) - g_option_context_free (context); - return ret; } diff --git a/src/ostree/ot-builtin-init.c b/src/ostree/ot-builtin-init.c index a250b793..8180a8ae 100644 --- a/src/ostree/ot-builtin-init.c +++ b/src/ostree/ot-builtin-init.c @@ -36,7 +36,7 @@ static GOptionEntry options[] = { gboolean ostree_builtin_init (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context = NULL; + g_autoptr(GOptionContext) context = NULL; g_autoptr(OstreeRepo) repo = NULL; gboolean ret = FALSE; OstreeRepoMode mode; @@ -54,7 +54,5 @@ ostree_builtin_init (int argc, char **argv, GCancellable *cancellable, GError ** ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-log.c b/src/ostree/ot-builtin-log.c index ab0f1f0d..ca1cdc68 100644 --- a/src/ostree/ot-builtin-log.c +++ b/src/ostree/ot-builtin-log.c @@ -81,7 +81,7 @@ ostree_builtin_log (int argc, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; const char *rev; @@ -111,7 +111,5 @@ ostree_builtin_log (int argc, ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-ls.c b/src/ostree/ot-builtin-ls.c index ab63c8bb..3abeb5c4 100644 --- a/src/ostree/ot-builtin-ls.c +++ b/src/ostree/ot-builtin-ls.c @@ -242,7 +242,7 @@ print_one_argument (OstreeRepo *repo, gboolean ostree_builtin_ls (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; const char *rev; @@ -280,7 +280,5 @@ ostree_builtin_ls (int argc, char **argv, GCancellable *cancellable, GError **er ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-prune.c b/src/ostree/ot-builtin-prune.c index 393ba379..d226c84b 100644 --- a/src/ostree/ot-builtin-prune.c +++ b/src/ostree/ot-builtin-prune.c @@ -169,7 +169,7 @@ gboolean ostree_builtin_prune (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; g_autofree char *formatted_freed_size = NULL; OstreeRepoPruneFlags pruneflags = 0; @@ -235,7 +235,5 @@ ostree_builtin_prune (int argc, char **argv, GCancellable *cancellable, GError * ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-pull-local.c b/src/ostree/ot-builtin-pull-local.c index 58a653ff..28717b85 100644 --- a/src/ostree/ot-builtin-pull-local.c +++ b/src/ostree/ot-builtin-pull-local.c @@ -53,7 +53,7 @@ gboolean ostree_builtin_pull_local (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; int i; const char *src_repo_arg; @@ -174,8 +174,6 @@ ostree_builtin_pull_local (int argc, char **argv, GCancellable *cancellable, GEr ret = TRUE; out: - if (context) - g_option_context_free (context); if (repo) ostree_repo_abort_transaction (repo, cancellable, NULL); return ret; diff --git a/src/ostree/ot-builtin-pull.c b/src/ostree/ot-builtin-pull.c index 8fa51002..f370ca01 100644 --- a/src/ostree/ot-builtin-pull.c +++ b/src/ostree/ot-builtin-pull.c @@ -109,7 +109,7 @@ dry_run_console_progress_changed (OstreeAsyncProgress *progress, gboolean ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; g_autofree char *remote = NULL; @@ -308,7 +308,5 @@ ostree_builtin_pull (int argc, char **argv, GCancellable *cancellable, GError ** out: if (signal_handler_id > 0) g_signal_handler_disconnect (repo, signal_handler_id); - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-refs.c b/src/ostree/ot-builtin-refs.c index 017b0c74..c7ea9a95 100644 --- a/src/ostree/ot-builtin-refs.c +++ b/src/ostree/ot-builtin-refs.c @@ -130,7 +130,7 @@ gboolean ostree_builtin_refs (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; int i; @@ -172,8 +172,6 @@ ostree_builtin_refs (int argc, char **argv, GCancellable *cancellable, GError ** ret = TRUE; out: - if (context) - g_option_context_free (context); if (repo) ostree_repo_abort_transaction (repo, cancellable, NULL); return ret; diff --git a/src/ostree/ot-builtin-remote.c b/src/ostree/ot-builtin-remote.c index 57c3ae09..4f01cac2 100644 --- a/src/ostree/ot-builtin-remote.c +++ b/src/ostree/ot-builtin-remote.c @@ -113,7 +113,7 @@ ostree_builtin_remote (int argc, char **argv, GCancellable *cancellable, GError if (!subcommand->name) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; g_autofree char *help; context = remote_option_context_new_with_commands (); @@ -137,8 +137,6 @@ ostree_builtin_remote (int argc, char **argv, GCancellable *cancellable, GError help = g_option_context_get_help (context, FALSE, NULL); g_printerr ("%s", help); - g_option_context_free (context); - goto out; } diff --git a/src/ostree/ot-builtin-reset.c b/src/ostree/ot-builtin-reset.c index 9468ae78..cab579ae 100644 --- a/src/ostree/ot-builtin-reset.c +++ b/src/ostree/ot-builtin-reset.c @@ -37,7 +37,7 @@ ostree_builtin_reset (int argc, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; g_autoptr(GHashTable) known_refs = NULL; gboolean ret = FALSE; @@ -83,8 +83,6 @@ ostree_builtin_reset (int argc, ret = TRUE; out: - if (context) - g_option_context_free (context); if (repo) ostree_repo_abort_transaction (repo, cancellable, NULL); return ret; diff --git a/src/ostree/ot-builtin-rev-parse.c b/src/ostree/ot-builtin-rev-parse.c index fb68d4ac..bedcc79d 100644 --- a/src/ostree/ot-builtin-rev-parse.c +++ b/src/ostree/ot-builtin-rev-parse.c @@ -34,7 +34,7 @@ static GOptionEntry options[] = { gboolean ostree_builtin_rev_parse (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; const char *rev = "master"; @@ -63,7 +63,5 @@ ostree_builtin_rev_parse (int argc, char **argv, GCancellable *cancellable, GErr ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-show.c b/src/ostree/ot-builtin-show.c index a9c1fbbc..6c45002a 100644 --- a/src/ostree/ot-builtin-show.c +++ b/src/ostree/ot-builtin-show.c @@ -252,7 +252,7 @@ print_if_found (OstreeRepo *repo, gboolean ostree_builtin_show (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; gboolean ret = FALSE; const char *rev; @@ -361,7 +361,5 @@ ostree_builtin_show (int argc, char **argv, GCancellable *cancellable, GError ** ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-static-delta.c b/src/ostree/ot-builtin-static-delta.c index e1c3bb48..c8843a65 100644 --- a/src/ostree/ot-builtin-static-delta.c +++ b/src/ostree/ot-builtin-static-delta.c @@ -110,7 +110,7 @@ ot_static_delta_builtin_list (int argc, char **argv, GCancellable *cancellable, gboolean ret = FALSE; g_autoptr(GPtrArray) delta_names = NULL; guint i; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; context = g_option_context_new ("LIST - list static delta files"); @@ -135,8 +135,6 @@ ot_static_delta_builtin_list (int argc, char **argv, GCancellable *cancellable, ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } @@ -144,7 +142,7 @@ static gboolean ot_static_delta_builtin_show (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *delta_id = NULL; @@ -167,8 +165,6 @@ ot_static_delta_builtin_show (int argc, char **argv, GCancellable *cancellable, ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } @@ -176,7 +172,7 @@ static gboolean ot_static_delta_builtin_delete (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *delta_id = NULL; @@ -199,8 +195,6 @@ ot_static_delta_builtin_delete (int argc, char **argv, GCancellable *cancellable ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } @@ -209,7 +203,7 @@ static gboolean ot_static_delta_builtin_generate (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; context = g_option_context_new ("GENERATE [TO] - Generate static delta files"); @@ -348,8 +342,6 @@ ot_static_delta_builtin_generate (int argc, char **argv, GCancellable *cancellab ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } @@ -359,7 +351,7 @@ ot_static_delta_builtin_apply_offline (int argc, char **argv, GCancellable *canc gboolean ret = FALSE; const char *patharg; g_autoptr(GFile) path = NULL; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; context = g_option_context_new ("APPLY-OFFLINE - Apply static delta file"); @@ -390,8 +382,6 @@ ot_static_delta_builtin_apply_offline (int argc, char **argv, GCancellable *canc ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-builtin-summary.c b/src/ostree/ot-builtin-summary.c index 2ff1c965..04ba84e8 100644 --- a/src/ostree/ot-builtin-summary.c +++ b/src/ostree/ot-builtin-summary.c @@ -40,7 +40,7 @@ gboolean ostree_builtin_summary (int argc, char **argv, GCancellable *cancellable, GError **error) { gboolean ret = FALSE; - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; context = g_option_context_new ("Manage summary metadata"); @@ -75,7 +75,5 @@ ostree_builtin_summary (int argc, char **argv, GCancellable *cancellable, GError ret = TRUE; out: - if (context) - g_option_context_free (context); return ret; } diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index 0d0587f2..fb782275 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -83,7 +83,7 @@ int ostree_usage (OstreeCommand *commands, gboolean is_error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; g_autofree char *help; context = ostree_option_context_new_with_commands (commands); @@ -97,8 +97,6 @@ ostree_usage (OstreeCommand *commands, else g_print ("%s", help); - g_option_context_free (context); - return (is_error ? 1 : 0); } @@ -172,7 +170,7 @@ ostree_run (int argc, if (!command->fn) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; g_autofree char *help; context = ostree_option_context_new_with_commands (commands); @@ -196,8 +194,6 @@ ostree_run (int argc, help = g_option_context_get_help (context, FALSE, NULL); g_printerr ("%s", help); - g_option_context_free (context); - goto out; } diff --git a/src/ostree/ot-remote-builtin-add-cookie.c b/src/ostree/ot-remote-builtin-add-cookie.c index f1657bb8..509c9c7a 100644 --- a/src/ostree/ot-remote-builtin-add-cookie.c +++ b/src/ostree/ot-remote-builtin-add-cookie.c @@ -37,7 +37,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_add_cookie (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *remote_name; const char *domain; @@ -80,7 +80,5 @@ ot_remote_builtin_add_cookie (int argc, char **argv, GCancellable *cancellable, /* jar takes ownership of cookie */ soup_cookie_jar_add_cookie (jar, cookie); - if (context) - g_option_context_free (context); return TRUE; } diff --git a/src/ostree/ot-remote-builtin-add.c b/src/ostree/ot-remote-builtin-add.c index 9d275cb5..93c989f9 100644 --- a/src/ostree/ot-remote-builtin-add.c +++ b/src/ostree/ot-remote-builtin-add.c @@ -44,7 +44,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *remote_name; const char *remote_url; @@ -149,7 +149,5 @@ ot_remote_builtin_add (int argc, char **argv, GCancellable *cancellable, GError ret = TRUE; out: - g_option_context_free (context); - return ret; } diff --git a/src/ostree/ot-remote-builtin-delete-cookie.c b/src/ostree/ot-remote-builtin-delete-cookie.c index df65b277..d974dd8d 100644 --- a/src/ostree/ot-remote-builtin-delete-cookie.c +++ b/src/ostree/ot-remote-builtin-delete-cookie.c @@ -37,7 +37,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_delete_cookie (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *remote_name; const char *domain; @@ -92,7 +92,5 @@ ot_remote_builtin_delete_cookie (int argc, char **argv, GCancellable *cancellabl if (!found) g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, "Cookie not found in jar"); - if (context) - g_option_context_free (context); return found; } diff --git a/src/ostree/ot-remote-builtin-delete.c b/src/ostree/ot-remote-builtin-delete.c index caa5cf8f..8a4dd5f3 100644 --- a/src/ostree/ot-remote-builtin-delete.c +++ b/src/ostree/ot-remote-builtin-delete.c @@ -35,7 +35,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_delete (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *remote_name; gboolean ret = FALSE; @@ -63,7 +63,5 @@ ot_remote_builtin_delete (int argc, char **argv, GCancellable *cancellable, GErr ret = TRUE; out: - g_option_context_free (context); - return ret; } diff --git a/src/ostree/ot-remote-builtin-gpg-import.c b/src/ostree/ot-remote-builtin-gpg-import.c index 4d2ad505..d1db06d0 100644 --- a/src/ostree/ot-remote-builtin-gpg-import.c +++ b/src/ostree/ot-remote-builtin-gpg-import.c @@ -93,7 +93,7 @@ out: gboolean ot_remote_builtin_gpg_import (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; g_autoptr(GInputStream) source_stream = NULL; const char *remote_name; @@ -136,7 +136,5 @@ ot_remote_builtin_gpg_import (int argc, char **argv, GCancellable *cancellable, ret = TRUE; out: - g_option_context_free (context); - return ret; } diff --git a/src/ostree/ot-remote-builtin-list-cookies.c b/src/ostree/ot-remote-builtin-list-cookies.c index 5ccc5d8d..1865fb07 100644 --- a/src/ostree/ot-remote-builtin-list-cookies.c +++ b/src/ostree/ot-remote-builtin-list-cookies.c @@ -37,7 +37,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_list_cookies (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *remote_name; g_autofree char *jar_path = NULL; diff --git a/src/ostree/ot-remote-builtin-list.c b/src/ostree/ot-remote-builtin-list.c index c42c44ba..faf8d8ad 100644 --- a/src/ostree/ot-remote-builtin-list.c +++ b/src/ostree/ot-remote-builtin-list.c @@ -33,7 +33,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_list (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; g_auto(GStrv) remotes = NULL; guint ii, n_remotes = 0; @@ -73,7 +73,5 @@ ot_remote_builtin_list (int argc, char **argv, GCancellable *cancellable, GError ret = TRUE; out: - g_option_context_free (context); - return ret; } diff --git a/src/ostree/ot-remote-builtin-refs.c b/src/ostree/ot-remote-builtin-refs.c index 4317c45f..9e742912 100644 --- a/src/ostree/ot-remote-builtin-refs.c +++ b/src/ostree/ot-remote-builtin-refs.c @@ -35,7 +35,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_refs (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *remote_name; gboolean ret = FALSE; @@ -78,9 +78,6 @@ ot_remote_builtin_refs (int argc, char **argv, GCancellable *cancellable, GError } ret = TRUE; - out: - g_option_context_free (context); - return ret; } diff --git a/src/ostree/ot-remote-builtin-summary.c b/src/ostree/ot-remote-builtin-summary.c index 4659dd4d..39321b53 100644 --- a/src/ostree/ot-remote-builtin-summary.c +++ b/src/ostree/ot-remote-builtin-summary.c @@ -39,7 +39,7 @@ static GOptionEntry option_entries[] = { gboolean ot_remote_builtin_summary (int argc, char **argv, GCancellable *cancellable, GError **error) { - GOptionContext *context; + g_autoptr(GOptionContext) context = NULL; glnx_unref_object OstreeRepo *repo = NULL; const char *remote_name; g_autoptr(GBytes) summary_bytes = NULL; @@ -120,9 +120,6 @@ ot_remote_builtin_summary (int argc, char **argv, GCancellable *cancellable, GEr } ret = TRUE; - out: - g_option_context_free (context); - return ret; } From 640e92ef373d7a5a3efcb09852b5c20d23592395 Mon Sep 17 00:00:00 2001 From: Dan Nicholson Date: Fri, 9 Dec 2016 06:37:47 -0600 Subject: [PATCH 46/55] repo: Fix annotations for remote_fetch_summary functions These are out parameters, so add the (out) annotation and switch (nullable) to (optional) since the latter is used for the purpose of optional out parameters. Closes: #629 Approved by: cgwalters --- src/libostree/ostree-repo-pull.c | 7 ++++--- src/libostree/ostree-repo.c | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 0d33c048..46724b85 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3137,9 +3137,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, * @self: Self * @name: name of a remote * @options: (nullable): A GVariant a{sv} with an extensible set of flags - * @out_summary: (nullable): return location for raw summary data, or %NULL - * @out_signatures: (nullable): return location for raw summary signature - * data, or %NULL + * @out_summary: (out) (optional): return location for raw summary data, or + * %NULL + * @out_signatures: (out) (optional): return location for raw summary + * signature data, or %NULL * @cancellable: a #GCancellable * @error: a #GError * diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index 8aa76dac..b113ccc0 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -1700,9 +1700,10 @@ out: * ostree_repo_remote_fetch_summary: * @self: Self * @name: name of a remote - * @out_summary: (nullable): return location for raw summary data, or %NULL - * @out_signatures: (nullable): return location for raw summary signature - * data, or %NULL + * @out_summary: (out) (optional): return location for raw summary data, or + * %NULL + * @out_signatures: (out) (optional): return location for raw summary + * signature data, or %NULL * @cancellable: a #GCancellable * @error: a #GError * From ef438c8d60489d707d79abfa9d7da0932bd86986 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Dec 2016 13:38:49 -0500 Subject: [PATCH 47/55] build: Error if glib isn't found This is a bit extracted from my work on ASAN. Closes: #625 Approved by: jlebon --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index ff27b0a0..4e004ca6 100644 --- a/configure.ac +++ b/configure.ac @@ -53,7 +53,7 @@ AS_IF([test "$YACC" != "bison -y"], [AC_MSG_ERROR([bison not found but required] PKG_PROG_PKG_CONFIG -AM_PATH_GLIB_2_0 +AM_PATH_GLIB_2_0(,,AC_MSG_ERROR([GLib not found])) dnl When bumping the gio-unix-2.0 dependency (or glib-2.0 in general), dnl remember to bump GLIB_VERSION_MIN_REQUIRED and From 17f264a4876dee43d97f14b777abdc328333f0c8 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Dec 2016 14:20:19 -0500 Subject: [PATCH 48/55] repo: Add unconfigured-state to remote config options This is a migration from the origin version. It's nicer to have it in the remote, since that's what one needs to change. Then tools don't need to mess with the origin file.o In fact in this scenario one can keep the "media source" like `file:///install/repo` or whatever, since conceptually that's where it came from. We're just providing a better error. Closes: https://github.com/ostreedev/ostree/issues/626 Closes: #627 Approved by: jlebon --- man/ostree.repo-config.xml | 5 +++++ src/libostree/ostree-repo-pull.c | 18 ++++++++++++++++++ src/libostree/ostree-sysroot-upgrader.c | 4 +++- tests/pull-test.sh | 11 ++++++++++- 4 files changed, 36 insertions(+), 2 deletions(-) diff --git a/man/ostree.repo-config.xml b/man/ostree.repo-config.xml index 5e0383a6..b4c276df 100644 --- a/man/ostree.repo-config.xml +++ b/man/ostree.repo-config.xml @@ -179,6 +179,11 @@ Boston, MA 02111-1307, USA. tls-ca-path Path to file containing trusted anchors instead of the system CA database. + + + unconfigured-state + If set, pulls from this remote will fail with the configured text. This is intended for OS vendors which have a subscription process to access content. + diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 46724b85..9c99dc4f 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -2458,6 +2458,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, } else { + g_autofree char *unconfigured_state = NULL; + pull_data->remote_name = g_strdup (remote_name_or_baseurl); /* Fetch GPG verification settings from remote if it wasn't already @@ -2471,6 +2473,22 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!ostree_repo_remote_get_gpg_verify_summary (self, pull_data->remote_name, &pull_data->gpg_verify_summary, error)) goto out; + + /* NOTE: If changing this, see the matching implementation in + * ostree-sysroot-upgrader.c + */ + if (!ostree_repo_get_remote_option (self, pull_data->remote_name, + "unconfigured-state", NULL, + &unconfigured_state, + error)) + goto out; + + if (unconfigured_state) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "remote unconfigured-state: %s", unconfigured_state); + goto out; + } } pull_data->phase = OSTREE_PULL_PHASE_FETCHING_REFS; diff --git a/src/libostree/ostree-sysroot-upgrader.c b/src/libostree/ostree-sysroot-upgrader.c index 447bd82b..daf2445c 100644 --- a/src/libostree/ostree-sysroot-upgrader.c +++ b/src/libostree/ostree-sysroot-upgrader.c @@ -77,7 +77,9 @@ parse_refspec (OstreeSysrootUpgrader *self, if ((self->flags & OSTREE_SYSROOT_UPGRADER_FLAGS_IGNORE_UNCONFIGURED) == 0) { - /* If explicit action by the OS creator is requried to upgrade, print their text as an error */ + /* If explicit action by the OS creator is requried to upgrade, print their text as an error. + * NOTE: If changing this, see the matching implementation in ostree-repo-pull.c. + */ unconfigured_state = g_key_file_get_string (self->origin, "origin", "unconfigured-state", NULL); if (unconfigured_state) { diff --git a/tests/pull-test.sh b/tests/pull-test.sh index 408d0539..56258b6c 100755 --- a/tests/pull-test.sh +++ b/tests/pull-test.sh @@ -35,7 +35,7 @@ function verify_initial_contents() { assert_file_has_content baz/cow '^moo$' } -echo "1..13" +echo "1..14" # Try both syntaxes repo_init @@ -249,3 +249,12 @@ assert_file_has_content baz/cow "further modified file for static deltas" assert_not_has_file baz/saucer echo "ok static delta 2" + +cd ${test_tmpdir} +${CMD_PREFIX} ostree --repo=repo remote add --set=gpg-verify=false --set=unconfigured-state="Access to ExampleOS requires ONE BILLION DOLLARS." origin-subscription file://$(pwd)/ostree-srv/gnomerepo +if ${CMD_PREFIX} ostree --repo=repo pull origin-subscription main 2>err.txt; then + assert_not_reached "pull unexpectedly succeeded?" +fi +assert_file_has_content err.txt "ONE BILLION DOLLARS" + +echo "ok unconfigured" From 47b4dd1b38e422254afa67756873957c25aeab6d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 7 Dec 2016 21:59:40 -0500 Subject: [PATCH 49/55] Skip gjs-based tests if ASAN is enabled Unfortunately, introspection uses dlopen(), which doesn't quite work when the DSO is compiled with ASAN but the outer executable isn't. Trying to inject LD_PRELOAD=libasan means the outer executable has to be leak free...which, yeah, I'm not going to get into running ASAN today on gjs or pygobject. So, let's skip those tests - ideally, we still run them in some other context without the sanitizers. The coverage we have from them is middling anyways. Closes: #622 Approved by: jlebon --- Makefile-tests.am | 10 ++++++---- configure.ac | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 5782fcbd..11ac2d7f 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -33,6 +33,9 @@ TESTS_ENVIRONMENT += OT_TESTS_DEBUG=1 \ LD_LIBRARY_PATH=$$(cd $(top_builddir)/.libs && pwd)$${LD_LIBRARY_PATH:+:$${LD_LIBRARY_PATH}} \ PATH=$$(cd $(top_builddir)/tests && pwd):$${PATH} \ $(NULL) +if BUILDOPT_ASAN +TESTS_ENVIRONMENT += OT_SKIP_READDIR_RAND=1 G_SLICE=always-malloc +endif uninstalled_test_data = tests/ostree-symlink-stamp tests/ostree-prepare-root-symlink-stamp \ tests/ostree-remount-symlink-stamp tests/rofiles-fuse-symlink-stamp @@ -53,7 +56,6 @@ dist_test_scripts = \ tests/test-parent.sh \ tests/test-pull-archive-z.sh \ tests/test-pull-commit-only.sh \ - tests/test-pull-corruption.sh \ tests/test-pull-depth.sh \ tests/test-pull-mirror-summary.sh \ tests/test-pull-large-metadata.sh \ @@ -104,10 +106,10 @@ if USE_LIBSOUP dist_test_scripts += tests/test-remote-cookies.sh endif -# This one uses corrupt-repo-ref.js -js_tests = tests/test-corruption.sh +# These call into gjs scripts +js_tests = tests/test-corruption.sh tests/test-pull-corruption.sh if BUILDOPT_GJS -dist_test_scripts += tests/test-corruption.sh +dist_test_scripts += $(js_tests) else EXTRA_DIST += $(js_tests) endif diff --git a/configure.ac b/configure.ac index 4e004ca6..7168221d 100644 --- a/configure.ac +++ b/configure.ac @@ -37,6 +37,15 @@ CC_CHECK_FLAGS_APPEND([WARN_CFLAGS], [CFLAGS], [\ ]) AC_SUBST(WARN_CFLAGS) +AC_MSG_CHECKING([for -fsanitize=address in CFLAGS]) +if echo $CFLAGS | grep -q -e -fsanitize=address; then + AC_MSG_RESULT([yes]) + using_asan=yes +else + AC_MSG_RESULT([no]) +fi +AM_CONDITIONAL(BUILDOPT_ASAN, [test x$using_asan = xyes]) + # Initialize libtool LT_PREREQ([2.2.4]) LT_INIT([disable-static]) @@ -305,8 +314,9 @@ AC_ARG_WITH(static-compiler, AM_CONDITIONAL(BUILDOPT_USE_STATIC_COMPILER, test x$with_static_compiler != xno) AC_SUBST(STATIC_COMPILER, $with_static_compiler) -dnl for tests -AS_IF([test "x$found_introspection" = xyes], [ +dnl for tests (but we can't use asan with gjs or any introspection, +dnl see https://github.com/google/sanitizers/wiki/AddressSanitizerAsDso for more info) +AS_IF([test "x$found_introspection" = xyes && test x$using_asan != xyes], [ AC_PATH_PROG(GJS, [gjs]) if test -n "$GJS"; then have_gjs=yes From 359ea9b58f789ad6a51e6f13e54d4034e6f30acd Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Dec 2016 12:50:20 -0500 Subject: [PATCH 50/55] tests: Tweak installed tests to deal with ASAN We need to disable readdir-rand there too. Closes: #622 Approved by: jlebon --- Makefile-tests.am | 11 ++++++++++- tests/libtest.sh | 5 ++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Makefile-tests.am b/Makefile-tests.am index 11ac2d7f..8c37a6e6 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -116,7 +116,6 @@ endif dist_installed_test_data = tests/archive-test.sh \ tests/pull-test.sh \ - tests/libtest.sh \ tests/admin-test.sh \ tests/basic-test.sh \ tests/test-basic-user.sh \ @@ -125,6 +124,8 @@ dist_installed_test_data = tests/archive-test.sh \ tests/pre-endian-deltas-repo-little.tar.xz \ $(NULL) +EXTRA_DIST += tests/libtest.sh + dist_test_extra_scripts = tests/bootloader-entries-crosscheck.py \ tests/ostree-grub-generator @@ -286,4 +287,12 @@ install-test-data-file-path-hack: fi ln -s . $(DESTDIR)$(installed_testdir)/tests INSTALL_DATA_HOOKS += install-test-data-file-path-hack + +install-libtest: +if BUILDOPT_ASAN + sed -e 's,^BUILT_WITH_ASAN=.*,BUILT_WITH_ASAN=1,' < $(srcdir)/tests/libtest.sh > $(DESTDIR)$(installed_testdir)/tests/libtest.sh +else + install -m 0644 $(srcdir)/tests/libtest.sh $(DESTDIR)$(installed_testdir)/tests/libtest.sh +endif +INSTALL_DATA_HOOKS += install-libtest endif diff --git a/tests/libtest.sh b/tests/libtest.sh index d16aae70..c0bf8d0d 100755 --- a/tests/libtest.sh +++ b/tests/libtest.sh @@ -86,13 +86,16 @@ if test -n "${OT_TESTS_DEBUG:-}"; then set -x fi +# This is substituted by the build for installed tests +BUILT_WITH_ASAN="" + if test -n "${OT_TESTS_VALGRIND:-}"; then CMD_PREFIX="env G_SLICE=always-malloc OSTREE_SUPPRESS_SYNCFS=1 valgrind -q --error-exitcode=1 --leak-check=full --num-callers=30 --suppressions=${test_srcdir}/glib.supp --suppressions=${test_srcdir}/ostree.supp" else # In some cases the LD_PRELOAD may cause obscure problems, # e.g. right now it breaks for me with -fsanitize=address, so # let's allow users to skip it. - if test -z "${OT_SKIP_READDIR_RAND:-}"; then + if test -z "${OT_SKIP_READDIR_RAND:-}" && test -z "${BUILT_WITH_ASAN:-}"; then CMD_PREFIX="env LD_PRELOAD=${test_builddir}/libreaddir-rand.so" else CMD_PREFIX="" From 67ce5ec91762887a7a80a4e725bdfdc7442edc73 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Dec 2016 13:35:29 -0500 Subject: [PATCH 51/55] ci: Drop sudo installed tests This conflicts with the ASAN work...and in general, I think I'd like to make a new format for tests that require root, and have them be defined to be in mutable containers or VMs. Our coverage loss from this isn't much because some of these tests already required `CAP_SYS_ADMIN` which we didn't have in Docker anyways. While we have the patient open, parallelize the regular installed tests. Closes: #622 Approved by: jlebon --- .redhat-ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.redhat-ci.yml b/.redhat-ci.yml index 4a3778ff..33e32995 100644 --- a/.redhat-ci.yml +++ b/.redhat-ci.yml @@ -21,8 +21,7 @@ build: tests: - make syntax-check - make check - - gnome-desktop-testing-runner ostree - - sudo --user=testuser gnome-desktop-testing-runner ostree + - gnome-desktop-testing-runner -p 0 ostree timeout: 30m From 5aefe17ee984e32e09bf24f2e3ef57cb78c7914c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 7 Dec 2016 21:26:43 -0500 Subject: [PATCH 52/55] ci: Combine UBSAN and ASAN by default I only recently realized this was possible. While we're still seeing leaks in the CI environment for some reason, adding ASAN gives us use-after-free detection etc., which is obviously still very useful even if we're not doing leak checking. Closes: #622 Approved by: jlebon --- .redhat-ci.Dockerfile | 2 ++ .redhat-ci.yml | 7 ++++++- Makefile-tests.am | 7 ++----- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.redhat-ci.Dockerfile b/.redhat-ci.Dockerfile index f1a41299..c78e5c4c 100644 --- a/.redhat-ci.Dockerfile +++ b/.redhat-ci.Dockerfile @@ -11,6 +11,8 @@ RUN dnf install -y \ parallel \ clang \ libubsan \ + libasan \ + libtsan \ gnome-desktop-testing \ redhat-rpm-config \ elfutils \ diff --git a/.redhat-ci.yml b/.redhat-ci.yml index 33e32995..5160b9c4 100644 --- a/.redhat-ci.yml +++ b/.redhat-ci.yml @@ -8,8 +8,13 @@ required: true container: image: projectatomic/ostree-tester +packages: + - libasan + env: - CFLAGS: '-fsanitize=undefined' + CFLAGS: '-fsanitize=undefined -fsanitize=address' + ASAN_OPTIONS: 'detect_leaks=0' # Right now we're not fully clean, but this gets us use-after-free etc + # TODO when we're doing leak checks: G_SLICE: "always-malloc" build: config-opts: > diff --git a/Makefile-tests.am b/Makefile-tests.am index 8c37a6e6..1f9cad48 100644 --- a/Makefile-tests.am +++ b/Makefile-tests.am @@ -281,18 +281,15 @@ tests/%-symlink-stamp: % Makefile # non-recursive Automake, so we change our code to canonically look # for tests/ which is just a symlink when installed. if ENABLE_INSTALLED_TESTS -install-test-data-file-path-hack: +install-installed-tests-extra: if test -L $(DESTDIR)$(installed_testdir)/tests; then \ rm $(DESTDIR)$(installed_testdir)/tests; \ fi ln -s . $(DESTDIR)$(installed_testdir)/tests -INSTALL_DATA_HOOKS += install-test-data-file-path-hack - -install-libtest: if BUILDOPT_ASAN sed -e 's,^BUILT_WITH_ASAN=.*,BUILT_WITH_ASAN=1,' < $(srcdir)/tests/libtest.sh > $(DESTDIR)$(installed_testdir)/tests/libtest.sh else install -m 0644 $(srcdir)/tests/libtest.sh $(DESTDIR)$(installed_testdir)/tests/libtest.sh endif -INSTALL_DATA_HOOKS += install-libtest +INSTALL_DATA_HOOKS += install-installed-tests-extra endif From 2f86a5284ca6415fe01bac83fa85e10b3ce99d85 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 8 Dec 2016 23:05:39 -0500 Subject: [PATCH 53/55] lib: Squash last use of GFile deltas_dir I was having this thought today about making more of the OS readonly, and ultimately if we got to the point where all ostree operations are through the repo and sysroot dfds, we could have rpm-ostree be the only process holding those fds open, and have a read-only bind mount on top. Anyways, we're not there, likely won't be soon, but this gets us closer to being fully fd relative. Closes: #628 Approved by: jlebon --- src/libostree/ostree-repo-private.h | 1 - src/libostree/ostree-repo-static-delta-core.c | 123 +++++++++--------- src/libostree/ostree-repo.c | 3 - 3 files changed, 64 insertions(+), 63 deletions(-) diff --git a/src/libostree/ostree-repo-private.h b/src/libostree/ostree-repo-private.h index 6b25a6fc..a4e59e44 100644 --- a/src/libostree/ostree-repo-private.h +++ b/src/libostree/ostree-repo-private.h @@ -72,7 +72,6 @@ struct OstreeRepo { int cache_dir_fd; char *cache_dir; int objects_dir_fd; - GFile *deltas_dir; int uncompressed_objects_dir_fd; GFile *sysroot_dir; char *remotes_config_dir; diff --git a/src/libostree/ostree-repo-static-delta-core.c b/src/libostree/ostree-repo-static-delta-core.c index 5ef4df90..edbf965b 100644 --- a/src/libostree/ostree-repo-static-delta-core.c +++ b/src/libostree/ostree-repo-static-delta-core.c @@ -73,95 +73,100 @@ ostree_repo_list_static_delta_names (OstreeRepo *self, GCancellable *cancellable, GError **error) { - gboolean ret = FALSE; g_autoptr(GPtrArray) ret_deltas = NULL; - g_autoptr(GFileEnumerator) dir_enum = NULL; + glnx_fd_close int dfd = -1; ret_deltas = g_ptr_array_new_with_free_func (g_free); - if (g_file_query_exists (self->deltas_dir, NULL)) + dfd = glnx_opendirat_with_errno (self->repo_dir_fd, "deltas", TRUE); + if (dfd < 0) { - dir_enum = g_file_enumerate_children (self->deltas_dir, OSTREE_GIO_FAST_QUERYINFO, - G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, - NULL, error); - if (!dir_enum) - goto out; + if (errno != ENOENT) + { + glnx_set_error_from_errno (error); + return FALSE; + } + } + else + { + g_auto(GLnxDirFdIterator) dfd_iter = { 0, }; + + if (!glnx_dirfd_iterator_init_take_fd (dfd, &dfd_iter, error)) + return FALSE; + dfd = -1; while (TRUE) { - g_autoptr(GFileEnumerator) dir_enum2 = NULL; - GFileInfo *file_info; - GFile *child; + g_auto(GLnxDirFdIterator) sub_dfd_iter = { 0, }; + struct dirent *dent; - if (!g_file_enumerator_iterate (dir_enum, &file_info, &child, - NULL, error)) - goto out; - if (file_info == NULL) + if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&dfd_iter, &dent, cancellable, error)) + return FALSE; + if (dent == NULL) break; - - if (g_file_info_get_file_type (file_info) != G_FILE_TYPE_DIRECTORY) + if (dent->d_type != DT_DIR) continue; - - dir_enum2 = g_file_enumerate_children (child, OSTREE_GIO_FAST_QUERYINFO, - G_FILE_QUERY_INFO_NOFOLLOW_SYMLINKS, - NULL, error); - if (!dir_enum2) - goto out; + if (!glnx_dirfd_iterator_init_at (dfd_iter.fd, dent->d_name, FALSE, + &sub_dfd_iter, error)) + return FALSE; while (TRUE) { - GFileInfo *file_info2; - GFile *child2; + struct dirent *sub_dent; const char *name1; const char *name2; + g_autofree char *superblock_subpath = NULL; + struct stat stbuf; - if (!g_file_enumerator_iterate (dir_enum2, &file_info2, &child2, - NULL, error)) - goto out; - if (file_info2 == NULL) + if (!glnx_dirfd_iterator_next_dent_ensure_dtype (&sub_dfd_iter, &sub_dent, + cancellable, error)) + return FALSE; + if (sub_dent == NULL) break; - - if (g_file_info_get_file_type (file_info2) != G_FILE_TYPE_DIRECTORY) + if (dent->d_type != DT_DIR) continue; - name1 = g_file_info_get_name (file_info); - name2 = g_file_info_get_name (file_info2); + name1 = dent->d_name; + name2 = sub_dent->d_name; - { - g_autoptr(GFile) meta_path = g_file_get_child (child2, "superblock"); + superblock_subpath = g_strconcat (name2, "/superblock", NULL); + if (fstatat (sub_dfd_iter.fd, superblock_subpath, &stbuf, 0) < 0) + { + if (errno != ENOENT) + { + glnx_set_error_from_errno (error); + return FALSE; + } + } + else + { + g_autofree char *buf = g_strconcat (name1, name2, NULL); + GString *out = g_string_new (""); + char checksum[OSTREE_SHA256_STRING_LEN+1]; + guchar csum[OSTREE_SHA256_DIGEST_LEN]; + const char *dash = strchr (buf, '-'); - if (g_file_query_exists (meta_path, NULL)) - { - g_autofree char *buf = g_strconcat (name1, name2, NULL); - GString *out = g_string_new (""); - char checksum[OSTREE_SHA256_STRING_LEN+1]; - guchar csum[OSTREE_SHA256_DIGEST_LEN]; - const char *dash = strchr (buf, '-'); + ostree_checksum_b64_inplace_to_bytes (buf, csum); + ostree_checksum_inplace_from_bytes (csum, checksum); + g_string_append (out, checksum); + if (dash) + { + g_string_append_c (out, '-'); + ostree_checksum_b64_inplace_to_bytes (dash+1, csum); + ostree_checksum_inplace_from_bytes (csum, checksum); + g_string_append (out, checksum); + } - ostree_checksum_b64_inplace_to_bytes (buf, csum); - ostree_checksum_inplace_from_bytes (csum, checksum); - g_string_append (out, checksum); - if (dash) - { - g_string_append_c (out, '-'); - ostree_checksum_b64_inplace_to_bytes (dash+1, csum); - ostree_checksum_inplace_from_bytes (csum, checksum); - g_string_append (out, checksum); - } - - g_ptr_array_add (ret_deltas, g_string_free (out, FALSE)); - } - } + g_ptr_array_add (ret_deltas, g_string_free (out, FALSE)); + } } } } - ret = TRUE; if (out_deltas) *out_deltas = g_steal_pointer (&ret_deltas); - out: - return ret; + return TRUE; } gboolean diff --git a/src/libostree/ostree-repo.c b/src/libostree/ostree-repo.c index b113ccc0..1c4ae663 100644 --- a/src/libostree/ostree-repo.c +++ b/src/libostree/ostree-repo.c @@ -521,7 +521,6 @@ ostree_repo_finalize (GObject *object) (void) close (self->cache_dir_fd); if (self->objects_dir_fd != -1) (void) close (self->objects_dir_fd); - g_clear_object (&self->deltas_dir); if (self->uncompressed_objects_dir_fd != -1) (void) close (self->uncompressed_objects_dir_fd); g_clear_object (&self->sysroot_dir); @@ -606,8 +605,6 @@ ostree_repo_constructed (GObject *object) self->tmp_dir = g_file_resolve_relative_path (self->repodir, "tmp"); - self->deltas_dir = g_file_get_child (self->repodir, "deltas"); - /* Ensure the "sysroot-path" property is set. */ if (self->sysroot_dir == NULL) self->sysroot_dir = g_object_ref (_ostree_get_default_sysroot_path ()); From 8bb8f0e3eb5e40bc9f25ed33bb8436214d80a3b2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 12 Dec 2016 09:05:06 -0500 Subject: [PATCH 54/55] ci: Rebase to f25 This is now the devel target, plus I think this may fix some of the ASAN issues I'm seeing; I mostly have been using f25 for local testing. Also remove the MAINTAINER line since the maintainers are defined by `git log`. Closes: #631 Approved by: jlebon --- .redhat-ci.Dockerfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.redhat-ci.Dockerfile b/.redhat-ci.Dockerfile index c78e5c4c..4970c77d 100644 --- a/.redhat-ci.Dockerfile +++ b/.redhat-ci.Dockerfile @@ -1,5 +1,4 @@ -FROM fedora:24 -MAINTAINER Jonathan Lebon +FROM fedora:25 RUN dnf install -y \ gcc \ From c9d565a5a9b435b5d3ed64a96c14a1c110698f4c Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 12 Dec 2016 11:54:03 -0500 Subject: [PATCH 55/55] Release 2016.15 Closes: #632 Approved by: jlebon --- configure.ac | 2 +- src/libostree/libostree.sym | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 7168221d..c59b3f7b 100644 --- a/configure.ac +++ b/configure.ac @@ -1,6 +1,6 @@ AC_PREREQ([2.63]) dnl If incrementing the version here, remember to update libostree.sym too -AC_INIT([ostree], [2016.14], [walters@verbum.org]) +AC_INIT([ostree], [2016.15], [walters@verbum.org]) AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree.sym b/src/libostree/libostree.sym index 0540cba6..0b8410fb 100644 --- a/src/libostree/libostree.sym +++ b/src/libostree/libostree.sym @@ -364,6 +364,8 @@ global: ostree_repo_verify_commit_for_remote; } LIBOSTREE_2016.8; +/* No new symbols in 2016.15 */ + /* NOTE NOTE NOTE * Versions above here are released. Only add symbols below this line. * NOTE NOTE NOTE