summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorMatthias Maennich <maennich@google.com>2021-11-26 11:06:42 +0000
committerDodji Seketeli <dodji@redhat.com>2022-01-19 14:48:03 +0100
commit194a3029bdf58101b402f2cfe7536768dd565c78 (patch)
treed5fb51f5ebf34fd709de64cd304374e59ce5beef
parentBug 28663 - generate man page for kmidiff (diff)
downloadlibabigail-194a3029bdf58101b402f2cfe7536768dd565c78.tar.gz
libabigail-194a3029bdf58101b402f2cfe7536768dd565c78.tar.bz2
libabigail-194a3029bdf58101b402f2cfe7536768dd565c78.tar.xz
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>
-rw-r--r--src/abg-reporter-priv.cc88
-rw-r--r--src/abg-reporter-priv.h2
-rw-r--r--tests/data/test-abidiff-exit/test-crc-report.txt6
-rw-r--r--tests/data/test-abidiff/test-crc-report.txt3
-rw-r--r--tests/data/test-diff-filter/test-PR27569-report-0.txt3
5 files changed, 25 insertions, 77 deletions
diff --git a/src/abg-reporter-priv.cc b/src/abg-reporter-priv.cc
index 3091f6ca..7012f5dc 100644
--- a/src/abg-reporter-priv.cc
+++ b/src/abg-reporter-priv.cc
@@ -1081,20 +1081,15 @@ maybe_report_diff_for_member(const decl_base_sptr& decl1,
1081/// @param the output stream to emit the report to. 1081/// @param the output stream to emit the report to.
1082/// 1082///
1083/// @param indent the indentation string to use. 1083/// @param indent the indentation string to use.
1084/// 1084void
1085/// @return true if a report was emitted to the output stream @p out,
1086/// false otherwise.
1087bool
1088maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1, 1085maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1,
1089 const elf_symbol_sptr& symbol2, 1086 const elf_symbol_sptr& symbol2,
1090 const diff_context_sptr& ctxt, 1087 const diff_context_sptr& ctxt,
1091 ostream& out, 1088 ostream& out,
1092 const string& indent) 1089 const string& indent)
1093{ 1090{
1094 bool reported = false;
1095
1096 if (!symbol1 || !symbol2 || symbol1 == symbol2) 1091 if (!symbol1 || !symbol2 || symbol1 == symbol2)
1097 return reported; 1092 return;
1098 1093
1099 if (symbol1->get_size() != symbol2->get_size()) 1094 if (symbol1->get_size() != symbol2->get_size())
1100 { 1095 {
@@ -1104,108 +1099,65 @@ maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1,
1104 symbol2->get_size(), 1099 symbol2->get_size(),
1105 *ctxt, out, 1100 *ctxt, out,
1106 /*show_bits_or_bytes=*/false); 1101 /*show_bits_or_bytes=*/false);
1107 reported = true; 1102 out << "\n";
1108 } 1103 }
1109 1104
1110 if (symbol1->get_name() != symbol2->get_name()) 1105 if (symbol1->get_name() != symbol2->get_name())
1111 { 1106 {
1112 if (reported) 1107 out << indent << "symbol name changed from "
1113 out << ",\n" << indent
1114 << "its name ";
1115 else
1116 out << "\n" << indent << "name of symbol ";
1117
1118 out << "changed from "
1119 << symbol1->get_name() 1108 << symbol1->get_name()
1120 << " to " 1109 << " to "
1121 << symbol2->get_name(); 1110 << symbol2->get_name()
1122 1111 << "\n";
1123 reported = true;
1124 } 1112 }
1125 1113
1126 if (symbol1->get_type() != symbol2->get_type()) 1114 if (symbol1->get_type() != symbol2->get_type())
1127 { 1115 {
1128 if (reported) 1116 out << indent << "symbol type changed from '"
1129 out << ",\n" << indent
1130 << "its type ";
1131 else
1132 out << "\n" << indent << "type of symbol ";
1133
1134 out << "changed from '"
1135 << symbol1->get_type() 1117 << symbol1->get_type()
1136 << "' to '" 1118 << "' to '"
1137 << symbol2->get_type() 1119 << symbol2->get_type()
1138 << "'"; 1120 << "'\n";
1139
1140 reported = true;
1141 } 1121 }
1142 1122
1143 if (symbol1->is_public() != symbol2->is_public()) 1123 if (symbol1->is_public() != symbol2->is_public())
1144 { 1124 {
1145 if (reported) 1125 out << indent << "symbol became ";
1146 out << ",\n" << indent
1147 << "it became ";
1148 else
1149 out << "\n" << indent << "symbol became ";
1150
1151 if (symbol2->is_public()) 1126 if (symbol2->is_public())
1152 out << "exported"; 1127 out << "exported";
1153 else 1128 else
1154 out << "non-exported"; 1129 out << "non-exported";
1155 1130 out << "\n";
1156 reported = true;
1157 } 1131 }
1158 1132
1159 if (symbol1->is_defined() != symbol2->is_defined()) 1133 if (symbol1->is_defined() != symbol2->is_defined())
1160 { 1134 {
1161 if (reported) 1135 out << indent << "symbol became ";
1162 out << ",\n" << indent
1163 << "it became ";
1164 else
1165 out << "\n" << indent << "symbol became ";
1166
1167 if (symbol2->is_defined()) 1136 if (symbol2->is_defined())
1168 out << "defined"; 1137 out << "defined";
1169 else 1138 else
1170 out << "undefined"; 1139 out << "undefined";
1171 1140 out << "\n";
1172 reported = true;
1173 } 1141 }
1174 1142
1175 if (symbol1->get_version() != symbol2->get_version()) 1143 if (symbol1->get_version() != symbol2->get_version())
1176 { 1144 {
1177 if (reported) 1145 out << indent << "symbol version changed from "
1178 out << ",\n" << indent 1146 << symbol1->get_version().str()
1179 << "its version changed from ";
1180 else
1181 out << "\n" << indent << "symbol version changed from ";
1182
1183 out << symbol1->get_version().str()
1184 << " to " 1147 << " to "
1185 << symbol2->get_version().str(); 1148 << symbol2->get_version().str()
1186 1149 << "\n";
1187 reported = true;
1188 } 1150 }
1189 1151
1190 if (symbol1->get_crc() != 0 && symbol2->get_crc() != 0 1152 if (symbol1->get_crc() != 0 && symbol2->get_crc() != 0
1191 && symbol1->get_crc() != symbol2->get_crc()) 1153 && symbol1->get_crc() != symbol2->get_crc())
1192 { 1154 {
1193 if (reported) 1155 out << indent << "CRC (modversions) changed from "
1194 out << ",\n" << indent << "its CRC (modversions) changed from "; 1156 << std::showbase << std::hex
1195 else
1196 out << "\n" << indent << "CRC value (modversions) changed from ";
1197
1198 out << std::showbase << std::hex
1199 << symbol1->get_crc() << " to " << symbol2->get_crc() 1157 << symbol1->get_crc() << " to " << symbol2->get_crc()
1200 << std::noshowbase << std::dec; 1158 << std::noshowbase << std::dec
1201 1159 << "\n";
1202 reported = true;
1203 } 1160 }
1204
1205 if (reported)
1206 out << "\n";
1207
1208 return reported;
1209} 1161}
1210 1162
1211/// For a given symbol, emit a string made of its name and version. 1163/// For a given symbol, emit a string made of its name and version.
diff --git a/src/abg-reporter-priv.h b/src/abg-reporter-priv.h
index 65786a0f..a7c4878c 100644
--- a/src/abg-reporter-priv.h
+++ b/src/abg-reporter-priv.h
@@ -206,7 +206,7 @@ maybe_report_diff_for_member(const decl_base_sptr& decl1,
206 ostream& out, 206 ostream& out,
207 const string& indent); 207 const string& indent);
208 208
209bool 209void
210maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1, 210maybe_report_diff_for_symbol(const elf_symbol_sptr& symbol1,
211 const elf_symbol_sptr& symbol2, 211 const elf_symbol_sptr& symbol2,
212 const diff_context_sptr& ctxt, 212 const diff_context_sptr& ctxt,
diff --git a/tests/data/test-abidiff-exit/test-crc-report.txt b/tests/data/test-abidiff-exit/test-crc-report.txt
index ddba41f4..2dbfa555 100644
--- a/tests/data/test-abidiff-exit/test-crc-report.txt
+++ b/tests/data/test-abidiff-exit/test-crc-report.txt
@@ -4,12 +4,10 @@ Variables changes summary: 0 Removed, 1 Changed, 0 Added variable
41 function with some indirect sub-type change: 41 function with some indirect sub-type change:
5 5
6 [C] 'function void func1(E)' has some indirect sub-type changes: 6 [C] 'function void func1(E)' has some indirect sub-type changes:
7 7 CRC (modversions) changed from 0x10000001 to 0x10000002
8 CRC value (modversions) changed from 0x10000001 to 0x10000002
9 8
101 Changed variable: 91 Changed variable:
11 10
12 [C] 'int var1' was changed: 11 [C] 'int var1' was changed:
13 12 CRC (modversions) changed from 0x30000001 to 0x30000002
14 CRC value (modversions) changed from 0x30000001 to 0x30000002
15 13
diff --git a/tests/data/test-abidiff/test-crc-report.txt b/tests/data/test-abidiff/test-crc-report.txt
index 4572a207..9bea309e 100644
--- a/tests/data/test-abidiff/test-crc-report.txt
+++ b/tests/data/test-abidiff/test-crc-report.txt
@@ -4,6 +4,5 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
41 function with some indirect sub-type change: 41 function with some indirect sub-type change:
5 5
6 [C] 'function void exported_function()' has some indirect sub-type changes: 6 [C] 'function void exported_function()' has some indirect sub-type changes:
7 7 CRC (modversions) changed from 0xe52d5bcf to 0xe52d5bd0
8 CRC value (modversions) changed from 0xe52d5bcf to 0xe52d5bd0
9 8
diff --git a/tests/data/test-diff-filter/test-PR27569-report-0.txt b/tests/data/test-diff-filter/test-PR27569-report-0.txt
index 419c9fd4..99febd66 100644
--- a/tests/data/test-diff-filter/test-PR27569-report-0.txt
+++ b/tests/data/test-diff-filter/test-PR27569-report-0.txt
@@ -4,7 +4,6 @@ Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
41 function with some indirect sub-type change: 41 function with some indirect sub-type change:
5 5
6 [C] 'function void device_add_disk(gendisk*)' at genhd.h:420:1 has some indirect sub-type changes: 6 [C] 'function void device_add_disk(gendisk*)' at genhd.h:420:1 has some indirect sub-type changes:
7 7 CRC (modversions) changed from 0x8afa957c to 0xa3bc03c
8 CRC value (modversions) changed from 0x8afa957c to 0xa3bc03c
9 parameter 2 of type 'int' was added 8 parameter 2 of type 'int' was added
10 9