[PATCH v2] abicompat: Add prompt message for abnormal operation

[PATCH v2] abicompat: Add prompt message for abnormal operation

[tangmeng]

When using abicompat, if the --redundant option and --no-redundant
option are used at the same time, no error will be prompted, the
actual option will prevail.

This patch provides a error to notify the user that error.

        * tools/abicompat.cc (parse_command_line): Notify the user when --redundant
        and --no-redundant are used at the same time

Signed-off-by: tangmeng <tangmeng@uniontech.com>
---
 tools/abicompat.cc | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/tools/abicompat.cc b/tools/abicompat.cc
index cf944a49..de5043ea 100644
--- a/tools/abicompat.cc
+++ b/tools/abicompat.cc
@@ -71,6 +71,8 @@ public:
   bool			list_undefined_symbols_only;
   bool			show_base_names;
   bool			show_redundant;
+  bool			seted_redundant;
+  bool			seted_no_redundant;
   bool			show_locs;
 
   options(const char* program_name)
@@ -191,9 +193,15 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  ++i;
 	}
       else if (!strcmp(argv[i], "--redundant"))
-	opts.show_redundant = true;
+        {
+	  opts.show_redundant = true;
+	  opts.seted_redundant = true;
+	}
       else if (!strcmp(argv[i], "--no-redundant"))
-	opts.show_redundant = false;
+        {
+  	  opts.show_redundant = false;
+	  opts.seted_no_redundant = true;
+	}
       else if (!strcmp(argv[i], "--no-show-locs"))
 	opts.show_locs = false;
       else if (!strcmp(argv[i], "--help")
@@ -645,6 +653,14 @@ main(int argc, char* argv[])
 	<< opts.lib2_path << " will be ignored automatically\n";
     }
 
+  if (opts.seted_redundant && opts.seted_no_redundant)
+    {
+      emit_prefix(argv[0], cerr)
+        << "ERROR: The \'--redundant\' and '--no-redundant' option are in conflict. "
+	<< "Please select only one option to use.\n";
+      return 1;
+    }
+
   ABG_ASSERT(!opts.app_path.empty());
   if (!abigail::tools_utils::check_file(opts.app_path, cerr, opts.prog_name))
     return abigail::tools_utils::ABIDIFF_ERROR;
-- 
2.20.1

Re: [PATCH v2] abicompat: Add prompt message for abnormal operation

[Dodji Seketeli]

Hello,

tangmeng <tangmeng@uniontech.com> a écrit:

> When using abicompat, if the --redundant option and --no-redundant
> option are used at the same time, no error will be prompted, the
> actual option will prevail.
>
> This patch provides a error to notify the user that error.
>
>         * tools/abicompat.cc (parse_command_line): Notify the user when --redundant
>         and --no-redundant are used at the same time
>
> Signed-off-by: tangmeng <tangmeng@uniontech.com>

Thanks for the patch!

I have applied a slightly modified version to yours that does
essentially the same thing.  Only minor nits have been picked.  Please
find the applied patch below.

Thanks again!

Cheers,

From 38d883cc7425cc945a8f51fd4cb66e90e2d8d825 Mon Sep 17 00:00:00 2001
From: tangmeng <tangmeng@uniontech.com>
Date: Fri, 5 Nov 2021 17:38:28 +0800
Subject: [PATCH] abicompat: Add prompt message for abnormal operation

When using abicompat, if the --redundant option and --no-redundant
option are used at the same time, no error is prompted and none of the
options have an impact.

This patch emits an error message in that case.

	* tools/abicompat.cc (parse_command_line): Notify the user
	when --redundant and --no-redundant are used at the same time

Signed-off-by: tangmeng <tangmeng@uniontech.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 tools/abicompat.cc | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/tools/abicompat.cc b/tools/abicompat.cc
index cf944a49..addd03a9 100644
--- a/tools/abicompat.cc
+++ b/tools/abicompat.cc
@@ -71,6 +71,8 @@ public:
   bool			list_undefined_symbols_only;
   bool			show_base_names;
   bool			show_redundant;
+  bool			redundant_opt_set;
+  bool			no_redundant_opt_set;
   bool			show_locs;
 
   options(const char* program_name)
@@ -81,6 +83,8 @@ public:
      list_undefined_symbols_only(),
      show_base_names(),
      show_redundant(true),
+     redundant_opt_set(),
+     no_redundant_opt_set(),
      show_locs(true)
   {}
 }; // end struct options
@@ -191,9 +195,15 @@ parse_command_line(int argc, char* argv[], options& opts)
 	  ++i;
 	}
       else if (!strcmp(argv[i], "--redundant"))
-	opts.show_redundant = true;
+        {
+	  opts.show_redundant = true;
+	  opts.redundant_opt_set = true;
+	}
       else if (!strcmp(argv[i], "--no-redundant"))
-	opts.show_redundant = false;
+        {
+  	  opts.show_redundant = false;
+	  opts.no_redundant_opt_set = true;
+	}
       else if (!strcmp(argv[i], "--no-show-locs"))
 	opts.show_locs = false;
       else if (!strcmp(argv[i], "--help")
@@ -645,6 +655,14 @@ main(int argc, char* argv[])
 	<< opts.lib2_path << " will be ignored automatically\n";
     }
 
+  if (opts.redundant_opt_set && opts.no_redundant_opt_set)
+    {
+      emit_prefix(argv[0], cerr)
+        << "ERROR: The \'--redundant\' and '--no-redundant' option are in conflict. "
+	<< "Please select only one option to use.\n";
+      return 1;
+    }
+
   ABG_ASSERT(!opts.app_path.empty());
   if (!abigail::tools_utils::check_file(opts.app_path, cerr, opts.prog_name))
     return abigail::tools_utils::ABIDIFF_ERROR;
-- 
2.32.0


-- 
		Dodji

Re: [PATCH v2] abicompat: Add prompt message for abnormal operation

[Dodji Seketeli]

Hello,

tangmeng <tangmeng@uniontech.com> a écrit:

> When using abicompat, if the user configures the
> lib2 path while using the xxx option, the configuration
> of lib2 is not actually processed in the command
> execution process, and there is no prompt message.
> It will cause lib2 to be considered to have been processed,
> so a warning is added here to prompt the user.

Thanks!  I have just re-worded this introduction a little bit.

[...]


> -/// An description of a change of the type of a function.  It contains
> +/// An description of a change of the type of a function. It contains

It's a common pattern we use throughout the code to add one extra space
after the dot.  This is convention used in the code of GNU projects and
we kept it for historical reasons.  So it's not a problem.  I changed
the "An description" into "A description", however.

So, here is the patch that was applied to the master branch of the Git
repository.

Thanks!

From ba18218ec2b8cf32b86aef4c4b539411ef0bd645 Mon Sep 17 00:00:00 2001
From: tangmeng <tangmeng@uniontech.com>
Date: Fri, 29 Oct 2021 17:23:43 +0800
Subject: [PATCH] abicompat: Add prompt message for abnormal operation

When using abicompat, if the uses the --weak-mode option and also
provides a lib2 path on the command line, the lib2 path is silently
ignored.

This patch provides a warning to notify the user that the lib2 path is
ignored in that case.

	* tools/abicompat.cc (main): Notify the user when the path to
	the second library is ignored because the --weak-mode option
	was provided.  Also, fix comment.

Signed-off-by: tangmeng <tangmeng@uniontech.com>
Signed-off-by: Dodji Seketeli <dodji@redhat.com>
---
 tools/abicompat.cc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/abicompat.cc b/tools/abicompat.cc
index 60cd794f..cf944a49 100644
--- a/tools/abicompat.cc
+++ b/tools/abicompat.cc
@@ -390,7 +390,7 @@ perform_compat_check_in_normal_mode(options& opts,
   return status;
 }
 
-/// An description of a change of the type of a function.  It contains
+/// A description of a change of the type of a function.  It contains
 /// the declaration of the function we are interested in, as well as
 /// the differences found in the type of that function.
 struct fn_change
@@ -638,6 +638,13 @@ main(int argc, char* argv[])
       return 0;
     }
 
+  if (opts.weak_mode && !opts.lib2_path.empty())
+    {
+      emit_prefix(argv[0], cout)
+        << "WARNING: The \'--weak-mode\' option is used. The "
+	<< opts.lib2_path << " will be ignored automatically\n";
+    }
+
   ABG_ASSERT(!opts.app_path.empty());
   if (!abigail::tools_utils::check_file(opts.app_path, cerr, opts.prog_name))
     return abigail::tools_utils::ABIDIFF_ERROR;
-- 
2.32.0



-- 
		Dodji

[PATCH v2] abicompat: Add prompt message for abnormal operation

[tangmeng]

When using abicompat, if the user configures the
lib2 path while using the xxx option, the configuration
of lib2 is not actually processed in the command
execution process, and there is no prompt message.
It will cause lib2 to be considered to have been processed,
so a warning is added here to prompt the user.

            * tools/abicompat.cc (main): Add warning tips for some actions
            (main): Remove extra spaces in the comment code

Signed-off-by: tangmeng <tangmeng@uniontech.com>
---
 tools/abicompat.cc | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tools/abicompat.cc b/tools/abicompat.cc
index 60cd794f..989915f3 100644
--- a/tools/abicompat.cc
+++ b/tools/abicompat.cc
@@ -390,7 +390,7 @@ perform_compat_check_in_normal_mode(options& opts,
   return status;
 }
 
-/// An description of a change of the type of a function.  It contains
+/// An description of a change of the type of a function. It contains
 /// the declaration of the function we are interested in, as well as
 /// the differences found in the type of that function.
 struct fn_change
@@ -638,6 +638,13 @@ main(int argc, char* argv[])
       return 0;
     }
 
+  if (opts.weak_mode && !opts.lib2_path.empty())
+    {
+      emit_prefix(argv[0], cout)
+        << "WARNING: The \'--weak-mode\' option is used. The "
+	<< opts.lib2_path << " will be ignored automatically\n";
+    }
+
   ABG_ASSERT(!opts.app_path.empty());
   if (!abigail::tools_utils::check_file(opts.app_path, cerr, opts.prog_name))
     return abigail::tools_utils::ABIDIFF_ERROR;
-- 
2.20.1