abidiff: improve whitespace generation in symbol diff report

maybe_report_diff_for_symbol() has a few issues:

1. The responsibility for newline emission is somewhat unclear and
   indeed it would emit spurious blank lines before most of the
   sub-diffs it reports.

2. Different sub-diff text and terminal commas are emitted according to
   whether or not there had been a previous sub-diff - making the output
   harder to grep and post-process.

3. The function also returns a bool but that return value is never used.

Hence, change the function to return void, the function stanzas to
always emit newline-terminated lines and ensure the wording and
punctuation of each sub-diff do not vary. This also tweaks (shortens)
the wording used for CRC diffs.

	* src/abg-reporter-priv.cc (maybe_report_diff_for_symbol):
	Make return void. Simplify and fix new-line emission. Remove
	comma emission. Tweak CRC wording.
	* src/abg-reporter-priv.h (maybe_report_diff_for_symbol):
	Return void.
	* tests/data/test-abidiff-exit/test-crc-report.txt: Shorten CRC
	wording.
	* tests/data/test-abidiff/test-crc-report.txt: Likewise.
	* tests/data/test-diff-filter/test-PR27569-report-0.txt:
	Likewise.

Reviewed-by: Giuliano Procida <gprocida@google.com>
Signed-off-by: Matthias Maennich <maennich@google.com>
This commit is contained in:
Matthias Maennich 2021-11-26 11:06:42 +00:00 committed by Dodji Seketeli
parent eb3e549794
commit 194a3029bd
5 changed files with 25 additions and 77 deletions

View File

@ -1081,20 +1081,15 @@ maybe_report_diff_for_member(const decl_base_sptr& decl1,
/// @param the output stream to emit the report to.
///
/// @param indent the indentation string to use.
///
/// @return true if a report was emitted to the output stream @p out,
/// false otherwise.
bool
void
maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1,
const elf_symbol_sptr& symbol2,
const diff_context_sptr& ctxt,
ostream& out,
const string& indent)
{
bool reported = false;
if (!symbol1 || !symbol2 || symbol1 == symbol2)
return reported;
return;
if (symbol1->get_size() != symbol2->get_size())
{
@ -1104,108 +1099,65 @@ maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1,
symbol2->get_size(),
*ctxt, out,
/*show_bits_or_bytes=*/false);
reported = true;
out << "\n";
}
if (symbol1->get_name() != symbol2->get_name())
{
if (reported)
out << ",\n" << indent
<< "its name ";
else
out << "\n" << indent << "name of symbol ";
out << "changed from "
out << indent << "symbol name changed from "
<< symbol1->get_name()
<< " to "
<< symbol2->get_name();
reported = true;
<< symbol2->get_name()
<< "\n";
}
if (symbol1->get_type() != symbol2->get_type())
{
if (reported)
out << ",\n" << indent
<< "its type ";
else
out << "\n" << indent << "type of symbol ";
out << "changed from '"
out << indent << "symbol type changed from '"
<< symbol1->get_type()
<< "' to '"
<< symbol2->get_type()
<< "'";
reported = true;
<< "'\n";
}
if (symbol1->is_public() != symbol2->is_public())
{
if (reported)
out << ",\n" << indent
<< "it became ";
else
out << "\n" << indent << "symbol became ";
out << indent << "symbol became ";
if (symbol2->is_public())
out << "exported";
else
out << "non-exported";
reported = true;
out << "\n";
}
if (symbol1->is_defined() != symbol2->is_defined())
{
if (reported)
out << ",\n" << indent
<< "it became ";
else
out << "\n" << indent << "symbol became ";
out << indent << "symbol became ";
if (symbol2->is_defined())
out << "defined";
else
out << "undefined";
reported = true;
out << "\n";
}
if (symbol1->get_version() != symbol2->get_version())
{
if (reported)
out << ",\n" << indent
<< "its version changed from ";
else
out << "\n" << indent << "symbol version changed from ";
out << symbol1->get_version().str()
out << indent << "symbol version changed from "
<< symbol1->get_version().str()
<< " to "
<< symbol2->get_version().str();
reported = true;
<< symbol2->get_version().str()
<< "\n";
}
if (symbol1->get_crc() != 0 && symbol2->get_crc() != 0
&& symbol1->get_crc() != symbol2->get_crc())
{
if (reported)
out << ",\n" << indent << "its CRC (modversions) changed from ";
else
out << "\n" << indent << "CRC value (modversions) changed from ";
out << std::showbase << std::hex
out << indent << "CRC (modversions) changed from "
<< std::showbase << std::hex
<< symbol1->get_crc() << " to " << symbol2->get_crc()
<< std::noshowbase << std::dec;
reported = true;
<< std::noshowbase << std::dec
<< "\n";
}
if (reported)
out << "\n";
return reported;
}
/// For a given symbol, emit a string made of its name and version.

View File

@ -206,7 +206,7 @@ maybe_report_diff_for_member(const decl_base_sptr& decl1,
ostream& out,
const string& indent);
bool
void
maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1,
const elf_symbol_sptr& symbol2,
const diff_context_sptr& ctxt,

View File

@ -4,12 +4,10 @@ Variables changes summary: 0 Removed, 1 Changed, 0 Added variable
1 function with some indirect sub-type change:
[C] 'function void func1(E)' has some indirect sub-type changes:
CRC value (modversions) changed from 0x10000001 to 0x10000002
CRC (modversions) changed from 0x10000001 to 0x10000002
1 Changed variable:
[C] 'int var1' was changed:
CRC value (modversions) changed from 0x30000001 to 0x30000002
CRC (modversions) changed from 0x30000001 to 0x30000002

View File

@ -4,6 +4,5 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
1 function with some indirect sub-type change:
[C] 'function void exported_function()' has some indirect sub-type changes:
CRC value (modversions) changed from 0xe52d5bcf to 0xe52d5bd0
CRC (modversions) changed from 0xe52d5bcf to 0xe52d5bd0

View File

@ -4,7 +4,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
1 function with some indirect sub-type change:
[C] 'function void device_add_disk(gendisk*)' at genhd.h:420:1 has some indirect sub-type changes:
CRC value (modversions) changed from 0x8afa957c to 0xa3bc03c
CRC (modversions) changed from 0x8afa957c to 0xa3bc03c
parameter 2 of type 'int' was added