From patchwork Fri Mar 27 19:01:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 39069 From: gprocida@google.com (Giuliano Procida) Date: Fri, 27 Mar 2020 19:01:35 +0000 Subject: [PATCH v2] Fix has_net_changes for --leaf-changes-only mode. In-Reply-To: <20200324171344.258865-1-gprocida@google.com> References: <20200324171344.258865-1-gprocida@google.com> Message-ID: <20200327190135.98611-1-gprocida@google.com> This function was not aware of --leaf-changes-only mode. - Stats counters for changed variables and types have different names in the different modes. - Leaf type changes were not included in leaf mode. For some inputs, this resulted in abidiff producing an empty report but returning a non-zero exit status in --leaf-changes-only mode. For other inputs, including some existing tests, the combination of both issues still resulted in the correct return code. This patch makes has_net_changes mirror emit_diff_stats, modulo flags like --non-reachable-types which if absent can still result in discrepancies between output and return code. * src/abg-comparison.cc (corpus_diff::priv::emit_diff_stats): Code whitespace. (corpus_diff::has_net_changes): Reorder the logic to match emit_diff_stats, add a note about this; in --leaf-changes-only mode use the right counters for function and variable changes and include type changes. Signed-off-by: Giuliano Procida Reviewed-by: Matthias Maennich --- src/abg-comparison.cc | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index 46bf9e30..50791cb5 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -9932,7 +9932,7 @@ corpus_diff::priv::emit_diff_stats(const diff_stats& s, out << " (" << s.num_changed_func_filtered_out() << " filtered out)"; out << ", "; - out << s.net_num_func_added()<< " Added"; + out << s.net_num_func_added() << " Added"; if (s.num_added_func_filtered_out()) out << " (" << s.num_added_func_filtered_out() << " filtered out)"; if (total_nb_function_changes <= 1) @@ -10560,7 +10560,7 @@ corpus_diff::has_incompatible_changes() const || stats.net_num_func_removed() != 0 || (stats.num_func_with_virtual_offset_changes() != 0 // If all reports about functions with sub-type changes - // have been suppressd, then even those about functions + // have been suppressed, then even those about functions // that are virtual don't matter anymore because the // user willingly requested to shut them down && stats.net_num_func_changed() != 0) @@ -10602,21 +10602,27 @@ corpus_diff::has_net_changes() const const diff_stats& stats = const_cast(this)-> apply_filters_and_suppressions_before_reporting(); + // Logic here should match emit_diff_stats. + // TODO: Possibly suppress things that won't be shown there. + bool leaf = context()->show_leaf_changes_only(); return (architecture_changed() || soname_changed() - || stats.net_num_func_changed() - || stats.net_num_vars_changed() - || stats.net_num_func_added() - || stats.net_num_added_func_syms() || stats.net_num_func_removed() - || stats.net_num_removed_func_syms() - || stats.net_num_vars_added() - || stats.net_num_added_var_syms() + || (leaf && stats.num_leaf_type_changes()) + || (leaf ? stats.net_num_leaf_func_changes() + : stats.net_num_func_changed()) + || stats.net_num_func_added() || stats.net_num_vars_removed() - || stats.net_num_removed_var_syms() - || stats.net_num_added_unreachable_types() + || (leaf ? stats.net_num_leaf_var_changes() + : stats.net_num_vars_changed()) + || stats.net_num_vars_added() || stats.net_num_removed_unreachable_types() - || stats.net_num_changed_unreachable_types()); + || stats.net_num_changed_unreachable_types() + || stats.net_num_added_unreachable_types() + || stats.net_num_removed_func_syms() + || stats.net_num_added_func_syms() + || stats.net_num_removed_var_syms() + || stats.net_num_added_var_syms()); } /// Apply the different filters that are registered to be applied to