* Re: [PATCH] abg-reader: handle empty corpus nodes in xml representation
2020-01-01 0:00 ` Dodji Seketeli
@ 2020-01-01 0:00 ` Matthias Maennich via libabigail
0 siblings, 0 replies; 3+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01 0:00 UTC (permalink / raw)
To: Dodji Seketeli; +Cc: libabigail, kernel-team
On Mon, Jan 20, 2020 at 10:49:55AM +0100, Dodji Seketeli wrote:
>Hello Matthias,
>
>Matthias Maennich <maennich@google.com> a écrit:
>
>> An abi-corpus might be part of the representation, but might (due to
>> filters like whitelisting) not contain actual symbols to be considered.
>> In that case, `abidw` produces an empty abi-corpus node.
>>
>> Valid ways of representing this in XML are
>>
>> - <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'/>
>>
>> - <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'></abi-corpus>
>>
>> - <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'>
>> </abi-corpus>
>>
>> abg-reader could currently only handle the last format and crashed upon
>> processing the first two ones. The crash happened due to the XMLNode
>> having no children, but that was assumed. The last case succeeded so
>> far as this form actually contains a text node (with the newline
>> character) as a child.
>>
>> Fix this by handling the case of a node not having children by exiting
>> early with an empty node.
>>
>> * src/abg-reader.cc (read_corpus_from_input): when assigning a
>> corpus node, assure the node actually has children.
>> * tests/test-abidiff.cc (main): Add test for variants of empty
>> xml nodes to the test harness.
>> * tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
>> containing an empty xml node that closes immediately.
>> * tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
>> containing an empty xml node that closes immediately with a tag.
>> * tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
>> containing an empty xml node that closes with a tag on a new line.
>> * tests/data/test-abidiff/test-empty-corpus-report.txt:
>> Expected test output (empty abidiff) for diffing xml with itself.
>> * tests/data/Makefile.am: Add the new test input material above
>> to source distribution.
>
>This is OK to commit to master.
Thanks! Applied to master.
Cheers,
Matthias
>
>Thank you for working on this!
>
>Cheers,
>
>--
> Dodji
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] abg-reader: handle empty corpus nodes in xml representation
2020-01-01 0:00 [PATCH] abg-reader: handle empty corpus nodes in xml representation Matthias Maennich via libabigail
@ 2020-01-01 0:00 ` Dodji Seketeli
2020-01-01 0:00 ` Matthias Maennich via libabigail
0 siblings, 1 reply; 3+ messages in thread
From: Dodji Seketeli @ 2020-01-01 0:00 UTC (permalink / raw)
To: Matthias Maennich; +Cc: libabigail, kernel-team
Hello Matthias,
Matthias Maennich <maennich@google.com> a écrit:
> An abi-corpus might be part of the representation, but might (due to
> filters like whitelisting) not contain actual symbols to be considered.
> In that case, `abidw` produces an empty abi-corpus node.
>
> Valid ways of representing this in XML are
>
> - <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'/>
>
> - <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'></abi-corpus>
>
> - <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'>
> </abi-corpus>
>
> abg-reader could currently only handle the last format and crashed upon
> processing the first two ones. The crash happened due to the XMLNode
> having no children, but that was assumed. The last case succeeded so
> far as this form actually contains a text node (with the newline
> character) as a child.
>
> Fix this by handling the case of a node not having children by exiting
> early with an empty node.
>
> * src/abg-reader.cc (read_corpus_from_input): when assigning a
> corpus node, assure the node actually has children.
> * tests/test-abidiff.cc (main): Add test for variants of empty
> xml nodes to the test harness.
> * tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
> containing an empty xml node that closes immediately.
> * tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
> containing an empty xml node that closes immediately with a tag.
> * tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
> containing an empty xml node that closes with a tag on a new line.
> * tests/data/test-abidiff/test-empty-corpus-report.txt:
> Expected test output (empty abidiff) for diffing xml with itself.
> * tests/data/Makefile.am: Add the new test input material above
> to source distribution.
This is OK to commit to master.
Thank you for working on this!
Cheers,
--
Dodji
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] abg-reader: handle empty corpus nodes in xml representation
@ 2020-01-01 0:00 Matthias Maennich via libabigail
2020-01-01 0:00 ` Dodji Seketeli
0 siblings, 1 reply; 3+ messages in thread
From: Matthias Maennich via libabigail @ 2020-01-01 0:00 UTC (permalink / raw)
To: libabigail; +Cc: dodji, kernel-team, maennich
An abi-corpus might be part of the representation, but might (due to
filters like whitelisting) not contain actual symbols to be considered.
In that case, `abidw` produces an empty abi-corpus node.
Valid ways of representing this in XML are
- <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'/>
- <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'></abi-corpus>
- <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'>
</abi-corpus>
abg-reader could currently only handle the last format and crashed upon
processing the first two ones. The crash happened due to the XMLNode
having no children, but that was assumed. The last case succeeded so
far as this form actually contains a text node (with the newline
character) as a child.
Fix this by handling the case of a node not having children by exiting
early with an empty node.
* src/abg-reader.cc (read_corpus_from_input): when assigning a
corpus node, assure the node actually has children.
* tests/test-abidiff.cc (main): Add test for variants of empty
xml nodes to the test harness.
* tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
containing an empty xml node that closes immediately.
* tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
containing an empty xml node that closes immediately with a tag.
* tests/data/test-abidiff/test-empty-corpus-0.xml: Test input
containing an empty xml node that closes with a tag on a new line.
* tests/data/test-abidiff/test-empty-corpus-report.txt:
Expected test output (empty abidiff) for diffing xml with itself.
* tests/data/Makefile.am: Add the new test input material above
to source distribution.
Signed-off-by: Matthias Maennich <maennich@google.com>
---
src/abg-reader.cc | 9 +++++----
tests/data/Makefile.am | 4 ++++
.../data/test-abidiff/test-empty-corpus-0.xml | 3 +++
.../data/test-abidiff/test-empty-corpus-1.xml | 3 +++
.../data/test-abidiff/test-empty-corpus-2.xml | 4 ++++
.../test-abidiff/test-empty-corpus-report.txt | 3 +++
tests/test-abidiff.cc | 18 ++++++++++++++++++
7 files changed, 40 insertions(+), 4 deletions(-)
create mode 100644 tests/data/test-abidiff/test-empty-corpus-0.xml
create mode 100644 tests/data/test-abidiff/test-empty-corpus-1.xml
create mode 100644 tests/data/test-abidiff/test-empty-corpus-2.xml
create mode 100644 tests/data/test-abidiff/test-empty-corpus-report.txt
diff --git a/src/abg-reader.cc b/src/abg-reader.cc
index eef6432650f7..5bf0d9ccdcb9 100644
--- a/src/abg-reader.cc
+++ b/src/abg-reader.cc
@@ -1867,8 +1867,6 @@ read_corpus_from_input(read_context& ctxt)
return nil;
call_reader_next = true;
-
- ctxt.set_corpus_node(node->children);
}
else
{
@@ -1898,10 +1896,13 @@ read_corpus_from_input(read_context& ctxt)
XML_NODE_GET_ATTRIBUTE(node, "soname");
if (soname_str)
corp.set_soname(reinterpret_cast<char*>(soname_str.get()));
-
- ctxt.set_corpus_node(node->children);
}
+ if (!node->children)
+ return nil;
+
+ ctxt.set_corpus_node(node->children);
+
corpus& corp = *ctxt.get_corpus();
walk_xml_node_to_map_type_ids(ctxt, node);
diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am
index 16026545fff4..adc95002d26e 100644
--- a/tests/data/Makefile.am
+++ b/tests/data/Makefile.am
@@ -84,6 +84,10 @@ test-abidiff/test-PR18791-v1.so.abi \
test-abidiff/test-PR24552-report0.txt \
test-abidiff/test-PR24552-v0.abi \
test-abidiff/test-PR24552-v1.abi \
+test-abidiff/test-empty-corpus-0.xml \
+test-abidiff/test-empty-corpus-1.xml \
+test-abidiff/test-empty-corpus-2.xml \
+test-abidiff/test-empty-corpus-report.txt \
\
test-abidiff-exit/test1-voffset-change-report0.txt \
test-abidiff-exit/test1-voffset-change-report1.txt \
diff --git a/tests/data/test-abidiff/test-empty-corpus-0.xml b/tests/data/test-abidiff/test-empty-corpus-0.xml
new file mode 100644
index 000000000000..aa8059bf956b
--- /dev/null
+++ b/tests/data/test-abidiff/test-empty-corpus-0.xml
@@ -0,0 +1,3 @@
+<abi-corpus-group architecture='elf-arm-aarch64'>
+ <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'/>
+</abi-corpus-group>
diff --git a/tests/data/test-abidiff/test-empty-corpus-1.xml b/tests/data/test-abidiff/test-empty-corpus-1.xml
new file mode 100644
index 000000000000..6adf18b9c097
--- /dev/null
+++ b/tests/data/test-abidiff/test-empty-corpus-1.xml
@@ -0,0 +1,3 @@
+<abi-corpus-group architecture='elf-arm-aarch64'>
+ <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'></abi-corpus>
+</abi-corpus-group>
diff --git a/tests/data/test-abidiff/test-empty-corpus-2.xml b/tests/data/test-abidiff/test-empty-corpus-2.xml
new file mode 100644
index 000000000000..5e025ff61648
--- /dev/null
+++ b/tests/data/test-abidiff/test-empty-corpus-2.xml
@@ -0,0 +1,4 @@
+<abi-corpus-group architecture='elf-arm-aarch64'>
+ <abi-corpus path='vmlinux' architecture='elf-arm-aarch64'>
+ </abi-corpus>
+</abi-corpus-group>
diff --git a/tests/data/test-abidiff/test-empty-corpus-report.txt b/tests/data/test-abidiff/test-empty-corpus-report.txt
new file mode 100644
index 000000000000..a9d032e74e35
--- /dev/null
+++ b/tests/data/test-abidiff/test-empty-corpus-report.txt
@@ -0,0 +1,3 @@
+Functions changes summary: 0 Removed, 0 Changed, 0 Added function
+Variables changes summary: 0 Removed, 0 Changed, 0 Added variable
+
diff --git a/tests/test-abidiff.cc b/tests/test-abidiff.cc
index d3118338157c..80e9c0ae5ee6 100644
--- a/tests/test-abidiff.cc
+++ b/tests/test-abidiff.cc
@@ -107,6 +107,24 @@ static InOutSpec specs[] =
"data/test-abidiff/test-PR24552-report0.txt",
"output/test-abidiff/test-PR24552-report0.txt"
},
+ {
+ "data/test-abidiff/test-empty-corpus-0.xml",
+ "data/test-abidiff/test-empty-corpus-0.xml",
+ "data/test-abidiff/test-empty-corpus-report.txt",
+ "output/test-abidiff/test-empty-corpus-report.txt"
+ },
+ {
+ "data/test-abidiff/test-empty-corpus-1.xml",
+ "data/test-abidiff/test-empty-corpus-1.xml",
+ "data/test-abidiff/test-empty-corpus-report.txt",
+ "output/test-abidiff/test-empty-corpus-report.txt"
+ },
+ {
+ "data/test-abidiff/test-empty-corpus-2.xml",
+ "data/test-abidiff/test-empty-corpus-2.xml",
+ "data/test-abidiff/test-empty-corpus-report.txt",
+ "output/test-abidiff/test-empty-corpus-report.txt"
+ },
// This should be the last entry.
{0, 0, 0, 0}
};
--
2.25.0.rc1.283.g88dfdc4193-goog
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-01-20 13:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-01 0:00 [PATCH] abg-reader: handle empty corpus nodes in xml representation Matthias Maennich via libabigail
2020-01-01 0:00 ` Dodji Seketeli
2020-01-01 0:00 ` Matthias Maennich via libabigail
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).