Bug 23533 - Accept '=' in ini property values
authorDodji Seketeli <dodji@redhat.com>
Thu, 30 Aug 2018 08:54:33 +0000 (10:54 +0200)
committerDodji Seketeli <dodji@redhat.com>
Thu, 30 Aug 2018 08:54:33 +0000 (10:54 +0200)
It appears that it's not possible to write a suppression specification
like the below at the moment:

  [suppress_file]
  label = Libabigail can't handle libgfortran.so (https://sourceware.org/bugzilla/show_bug.cgi?id=23492)
  file_name_regexp = libgfortran\\.so.*

This is because the ini parser won't accept the '=' character in the
URL as a valid character for ini property values.

So the entire [suppress_file] section is ignored by the suppression
specification engine.

This patch fixes that by making the equal character valid in property
values.

* src/abg-ini.cc (char_is_delimiter): Take a new include_equal
flag to control is the equal character should be considered as a
delimiter or not.
(char_is_property_value_char): Accept the equal character as a
valid property value character.
* tests/Makefile.am: Build a new runtestini test from the new
tests/test-ini.cc source file.
* tests/data/Makefile.am: Add the two new test inputs below to
source distribution.
* tests/data/test-ini/test01-equal-in-property-string.{abignore,
abignore.expected}: New test inputs.
* tests/test-ini.cc: New test harness.

Signed-off-by: Dodji Seketeli <dodji@redhat.com>
src/abg-ini.cc
tests/Makefile.am
tests/data/Makefile.am
tests/data/test-ini/test01-equal-in-property-string.abignore [new file with mode: 0644]
tests/data/test-ini/test01-equal-in-property-string.abignore.expected [new file with mode: 0644]
tests/test-ini.cc [new file with mode: 0644]

index d2a1abf..e60bcb4 100644 (file)
@@ -66,16 +66,20 @@ char_is_comment_start(int b);
 ///@param include_square_bracket if true, consider square brackets as
 /// delimiters
 ///
+/// @param include_equal if true, consider the equal character ('=')
+/// as a delimiter.
+///
 /// @return true iff @p b is a delimiter.
 static bool
 char_is_delimiter(int b, bool include_white_space = true,
-                 bool include_square_bracket = true)
+                 bool include_square_bracket = true,
+                 bool include_equal = true)
 {
   return ((include_square_bracket && (b == '['))
          || (include_square_bracket && (b == ']'))
          || b == '{'
          || b == '}'
-         || b == '='
+         || (include_equal && (b == '='))
          || b == ','
          || (include_white_space && char_is_white_space(b))
          || char_is_comment_start(b));
@@ -84,6 +88,10 @@ char_is_delimiter(int b, bool include_white_space = true,
 /// Return true iff a given character can be part of a property
 /// value.
 ///
+/// Note that white spaces, square brackets and the equal character can be
+/// part of a property value.  The reason why we accept the equal
+/// character is because it can appear in an URL.
+///
 /// @param b the character to test against.
 ///
 /// @return true iff @p b is a character that can be part of a
@@ -92,7 +100,8 @@ static bool
 char_is_property_value_char(int b)
 {
   if (char_is_delimiter(b, /*include_white_space=*/false,
-                       /*include_square_bracket=*/false)
+                       /*include_square_bracket=*/false,
+                       /*include_equal=*/false)
       || b == '\n')
     return false;
   return true;
index fcdbbd1..cae02a3 100644 (file)
@@ -44,6 +44,7 @@ runtestlookupsyms             \
 runtestaltdwarf                        \
 runtestcorediff                        \
 runtestabidiffexit             \
+runtestini                     \
 $(CXX11_TESTS)
 
 if ENABLE_RUNNING_TESTS_WITH_PY3
@@ -129,6 +130,9 @@ runtestdiffpkg_LDADD = libtestutils.la $(top_builddir)/src/libabigail.la
 runtesttypesstability_SOURCES = test-types-stability.cc
 runtesttypesstability_LDADD = libtestutils.la $(top_builddir)/src/libabigail.la
 
+runtestini_SOURCES = test-ini.cc
+runtestini_LDADD = libtestutils.la $(top_builddir)/src/libabigail.la
+
 runtestsvg_SOURCES=test-svg.cc
 runtestsvg_LDADD=$(top_builddir)/src/libabigail.la
 
index eeb2990..3709a4d 100644 (file)
@@ -1457,4 +1457,7 @@ test-fedabipkgdiff/dbus-glib/dbus-glib-0.100.2-2.fc20.x86_64.rpm \
 test-fedabipkgdiff/dbus-glib/dbus-glib-0.106-1.fc23.x86_64.rpm \
 test-fedabipkgdiff/nss-util/nss-util-devel-3.24.0-2.0.fc25.x86_64.rpm \
 test-fedabipkgdiff/nss-util/nss-util-3.12.6-1.fc14.x86_64.rpm \
-test-fedabipkgdiff/nss-util/nss-util-3.24.0-2.0.fc25.x86_64.rpm
+test-fedabipkgdiff/nss-util/nss-util-3.24.0-2.0.fc25.x86_64.rpm \
+\
+test-ini/test01-equal-in-property-string.abignore.expected \
+test-ini/test01-equal-in-property-string.abignore
diff --git a/tests/data/test-ini/test01-equal-in-property-string.abignore b/tests/data/test-ini/test01-equal-in-property-string.abignore
new file mode 100644 (file)
index 0000000..8c70b46
--- /dev/null
@@ -0,0 +1,4 @@
+[suppress_file]
+  # Somme comment
+  label = Libabigail can't handle libgfortran.so (https://sourceware.org/bugzilla/show_bug.cgi?id=23492)
+  file_name_regexp = libgfortran\\.so.*
\ No newline at end of file
diff --git a/tests/data/test-ini/test01-equal-in-property-string.abignore.expected b/tests/data/test-ini/test01-equal-in-property-string.abignore.expected
new file mode 100644 (file)
index 0000000..2c60162
--- /dev/null
@@ -0,0 +1,4 @@
+[suppress_file]
+  label = Libabigail can't handle libgfortran.so (https://sourceware.org/bugzilla/show_bug.cgi?id=23492)
+  file_name_regexp = libgfortran\.so.*
+
diff --git a/tests/test-ini.cc b/tests/test-ini.cc
new file mode 100644 (file)
index 0000000..dc55e06
--- /dev/null
@@ -0,0 +1,147 @@
+// -*- Mode: C++ -*-
+//
+// Copyright (C) 2013-2018 Red Hat, Inc.
+//
+// This file is part of the GNU Application Binary Interface Generic
+// Analysis and Instrumentation Library (libabigail).  This library is
+// free software; you can redistribute it and/or modify it under the
+// terms of the GNU Lesser General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option) any
+// later version.
+
+// This library is distributed in the hope that it will be useful, but
+// WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+// General Lesser Public License for more details.
+
+// You should have received a copy of the GNU Lesser General Public
+// License along with this program; see the file COPYING-LGPLV3.  If
+// not, see <http://www.gnu.org/licenses/>.
+
+// Author: Dodji Seketeli
+
+/// @file
+///
+/// This test harness program runs a diff between the output of
+/// abinilint on an ini file and a reference expected output.
+
+#include <sys/wait.h>
+#include <cstring>
+#include <string>
+#include <fstream>
+#include <iostream>
+#include <cstdlib>
+#include "abg-tools-utils.h"
+#include "test-utils.h"
+
+using std::string;
+using std::cerr;
+
+/// This is an aggregate that specifies where a test shall get its
+/// input from and where it shall write its ouput to.
+struct InOutSpec
+{
+  // Where to read the ini file from, with abinilint.
+  const char* in_ini_path;
+  // Where to get the expected output from abinilint.
+  const char* in_reference_output_path;
+  // Where to emit the output of abinilint to.
+  const char* out_init_path;
+  // The options to use with abinilint.
+  const char* abinilint_options;
+}; // end struct InOutSpec;
+
+// An array of test specifications listing the ini files to read and
+// their expected output.
+InOutSpec in_out_specs[] =
+{
+  {
+    "data/test-ini/test01-equal-in-property-string.abignore",
+    "data/test-ini/test01-equal-in-property-string.abignore.expected",
+    "output/test-ini/test01-equal-in-property-string.abignore",
+    ""
+  }
+  ,
+  // This one must always remain the last one.
+  {0, 0, 0, 0}
+};
+
+/// @return the test source directory.
+static string
+get_test_src_dir()
+{
+  using abigail::tests::get_src_dir;
+  return string(get_src_dir()) + "/tests";
+}
+
+/// @return the test build directory.
+static string
+get_test_build_dir()
+{
+  using abigail::tests::get_build_dir;
+  return string(get_build_dir()) + "/tests";
+}
+
+/// @return the tools directory under the build directory;
+static string
+get_tools_build_dir()
+{
+  using abigail::tests::get_build_dir;
+  return string(get_build_dir()) + "/tools";
+}
+
+int
+main()
+{
+  using abigail::tests::get_build_dir;
+  using abigail::tools_utils::ensure_parent_dir_created;
+  using abigail::tools_utils::abidiff_status;
+
+  bool is_ok = true;
+  string in_ini_path, in_reference_output_path, out_ini_path, cmd;
+
+  for (InOutSpec *s = in_out_specs; s->in_ini_path; ++s)
+    {
+      in_ini_path = get_test_src_dir() + "/" + s->in_ini_path;
+      in_reference_output_path =
+       get_test_src_dir() + "/" + s->in_reference_output_path;
+      out_ini_path = get_test_build_dir() + "/" + s->out_init_path;
+
+      if (!ensure_parent_dir_created(out_ini_path))
+       {
+         cerr << "could not create parent directory for "
+              << out_ini_path;
+         is_ok = false;
+         continue;
+       }
+
+      cmd = get_tools_build_dir() + "/abinilint";
+      if (s->abinilint_options && strcmp(s->abinilint_options, ""))
+       cmd += " " + string(s->abinilint_options);
+
+      cmd += " " + in_ini_path + " > " + out_ini_path;
+
+      bool cmd_is_ok = true;
+      int code = system(cmd.c_str());
+      if (!WIFEXITED(code))
+        cmd_is_ok = false;
+      else
+       {
+         abigail::tools_utils::abidiff_status status =
+           static_cast<abidiff_status>(WEXITSTATUS(code));
+         if (abigail::tools_utils::abidiff_status_has_error(status))
+           cmd_is_ok = false;
+       }
+
+      if (cmd_is_ok)
+       {
+         cmd = "diff -u " + in_reference_output_path + " " + out_ini_path;
+         if (system(cmd.c_str()))
+           is_ok = false;
+       }
+      else
+       is_ok = false;
+    }
+
+  return !is_ok;
+}