From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x343.google.com (mail-wm1-x343.google.com [IPv6:2a00:1450:4864:20::343]) by sourceware.org (Postfix) with ESMTPS id 78EF8385E030 for ; Mon, 30 Mar 2020 09:17:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 78EF8385E030 Received: by mail-wm1-x343.google.com with SMTP id j19so2220463wmi.2 for ; Mon, 30 Mar 2020 02:17:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ATAYt8m6heiGlJUnv54qoQ2ObORMQPLJ9Y3XHKWg2lk=; b=OabeY7pc/qSGwEC/0xzT+FtGVAM3K+ZsrEIUPcjb4/3DBDv102zukVvhU/P80cXuM2 YonRAVwX3yutxsEEuf5TzsZfK4uvzVEKilTxMoOKB7+f8s3CeoG0cBkBd1wOGdDQTvmN RAwuNPz6IMXqRKAT32hSx9k/75ETKa7k+9k+LRJoedcuT1NXf88X7ym3aekHGGdjfAOP FygFXtZ7edABjH+Y7MVrskstW7YmHjnXOpi+GU391kUcp1JW067v0Y6cFj2lTQJyaCOK k89I5i4I68vMz8ypc5A/hMB8ml0soBa7+yi44co6xQRSdsL/47dABxa6+TxCGg46i2W4 90/A== X-Gm-Message-State: ANhLgQ0WooXgyz4pM+vug+PNn+ajfKWPuyDwmLcahL6+rqHHBkrAYWh4 tpR5eTPVZhfoQ9CfRtyFry5NUw== X-Google-Smtp-Source: ADFU+vv6/2/kjd29hwQynvEOAWkRuYDidQYmlEwUZ1MGeNPvQXbDPxrJ3c2yr50ovPvKDgY0MzVyYw== X-Received: by 2002:a05:600c:2294:: with SMTP id 20mr11966374wmf.130.1585559875181; Mon, 30 Mar 2020 02:17:55 -0700 (PDT) Received: from google.com ([2a00:79e0:d:210:e8f7:125b:61e9:733d]) by smtp.gmail.com with ESMTPSA id v11sm21545387wrm.43.2020.03.30.02.17.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Mar 2020 02:17:54 -0700 (PDT) Date: Mon, 30 Mar 2020 11:17:54 +0200 From: Matthias Maennich To: Giuliano Procida Cc: libabigail@sourceware.org, dodji@seketeli.org, kernel-team@android.com Subject: Re: [PATCH v2] Fix has_net_changes for --leaf-changes-only mode. Message-ID: <20200330091754.GH101337@google.com> References: <20200324171344.258865-1-gprocida@google.com> <20200327190135.98611-1-gprocida@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20200327190135.98611-1-gprocida@google.com> User-Agent: Mutt/1.12.2 (2019-09-21) X-Spam-Status: No, score=-40.6 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libabigail mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Mar 2020 09:17:58 -0000 On Fri, Mar 27, 2020 at 07:01:35PM +0000, Giuliano Procida wrote: >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 Would feel better to have some tests covering this along with the patch. But this still looks correct to me. Reviewed-by: Matthias Maennich Cheers, Matthias >--- > 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 >-- >2.26.0.rc2.310.g2932bb562d-goog >