[libgpiod] [PATCH 1/5] bindings: cxx: Use 'upstream' include path

[libgpiod] [PATCH 1/5] bindings: cxx: Use 'upstream' include path

[Alexander Stein]

According to https://github.com/catchorg/Catch2/issues/1202 the
regular include is 'catch2/catch.hpp'
Also CMake and pkg-config provide include paths for this include name.

Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
---
 bindings/cxx/tests/gpiod-cxx-test.cpp | 2 +-
 bindings/cxx/tests/tests-chip.cpp     | 2 +-
 bindings/cxx/tests/tests-event.cpp    | 2 +-
 bindings/cxx/tests/tests-iter.cpp     | 2 +-
 bindings/cxx/tests/tests-line.cpp     | 2 +-
 configure.ac                          | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/bindings/cxx/tests/gpiod-cxx-test.cpp b/bindings/cxx/tests/gpiod-cxx-test.cpp
index fbae84f..236fd2d 100644
--- a/bindings/cxx/tests/gpiod-cxx-test.cpp
+++ b/bindings/cxx/tests/gpiod-cxx-test.cpp
@@ -6,7 +6,7 @@
  */
 
 #define CATCH_CONFIG_MAIN
-#include <catch.hpp>
+#include <catch2/catch.hpp>
 #include <linux/version.h>
 #include <sys/utsname.h>
 #include <system_error>
diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
index 276b533..11c2d4c 100644
--- a/bindings/cxx/tests/tests-chip.cpp
+++ b/bindings/cxx/tests/tests-chip.cpp
@@ -5,7 +5,7 @@
  * Copyright (C) 2019 Bartosz Golaszewski <bgolaszewski@baylibre.com>
  */
 
-#include <catch.hpp>
+#include <catch2/catch.hpp>
 #include <gpiod.hpp>
 
 #include "gpio-mockup.hpp"
diff --git a/bindings/cxx/tests/tests-event.cpp b/bindings/cxx/tests/tests-event.cpp
index f93bb72..b34347f 100644
--- a/bindings/cxx/tests/tests-event.cpp
+++ b/bindings/cxx/tests/tests-event.cpp
@@ -5,7 +5,7 @@
  * Copyright (C) 2019 Bartosz Golaszewski <bgolaszewski@baylibre.com>
  */
 
-#include <catch.hpp>
+#include <catch2/catch.hpp>
 #include <gpiod.hpp>
 #include <map>
 #include <poll.h>
diff --git a/bindings/cxx/tests/tests-iter.cpp b/bindings/cxx/tests/tests-iter.cpp
index 1af0256..fdc2cb5 100644
--- a/bindings/cxx/tests/tests-iter.cpp
+++ b/bindings/cxx/tests/tests-iter.cpp
@@ -5,7 +5,7 @@
  * Copyright (C) 2019 Bartosz Golaszewski <bgolaszewski@baylibre.com>
  */
 
-#include <catch.hpp>
+#include <catch2/catch.hpp>
 #include <gpiod.hpp>
 
 #include "gpio-mockup.hpp"
diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
index 2684bcb..e827e60 100644
--- a/bindings/cxx/tests/tests-line.cpp
+++ b/bindings/cxx/tests/tests-line.cpp
@@ -5,7 +5,7 @@
  * Copyright (C) 2019 Bartosz Golaszewski <bgolaszewski@baylibre.com>
  */
 
-#include <catch.hpp>
+#include <catch2/catch.hpp>
 #include <gpiod.hpp>
 
 #include "gpio-mockup.hpp"
diff --git a/configure.ac b/configure.ac
index f72e13b..0459007 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,7 +165,7 @@ then
 	if test "x$with_tests" = xtrue
 	then
 		AC_LANG_PUSH([C++])
-		AC_CHECK_HEADERS([catch.hpp], [], [HEADER_NOT_FOUND_CXX([catch.hpp])])
+		AC_CHECK_HEADERS([catch2/catch.hpp], [], [HEADER_NOT_FOUND_CXX([catch2/catch.hpp])])
 		AC_LANG_POP([C++])
 	fi
 fi
-- 
2.22.0

[libgpiod] [PATCH 2/5] bindings: cxx: Try using pkg-config to detect catch2

[Alexander Stein]

If there is no system wide package, try using a regular header as before.

Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
---
 bindings/cxx/tests/Makefile.am | 2 +-
 configure.ac                   | 8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/bindings/cxx/tests/Makefile.am b/bindings/cxx/tests/Makefile.am
index 155445f..d1da0d3 100644
--- a/bindings/cxx/tests/Makefile.am
+++ b/bindings/cxx/tests/Makefile.am
@@ -8,7 +8,7 @@
 
 AM_CPPFLAGS = -I$(top_srcdir)/bindings/cxx/ -I$(top_srcdir)/include
 AM_CPPFLAGS += -I$(top_srcdir)/tests/mockup/
-AM_CPPFLAGS += -Wall -Wextra -g -std=gnu++11
+AM_CPPFLAGS += -Wall -Wextra -g -std=gnu++11 $(CATCH2_CFLAGS)
 AM_LDFLAGS = -lgpiodcxx -L$(top_builddir)/bindings/cxx/
 AM_LDFLAGS += -lgpiomockup -L$(top_builddir)/tests/mockup/
 AM_LDFLAGS += -pthread
diff --git a/configure.ac b/configure.ac
index 0459007..bf364e7 100644
--- a/configure.ac
+++ b/configure.ac
@@ -164,9 +164,11 @@ then
 
 	if test "x$with_tests" = xtrue
 	then
-		AC_LANG_PUSH([C++])
-		AC_CHECK_HEADERS([catch2/catch.hpp], [], [HEADER_NOT_FOUND_CXX([catch2/catch.hpp])])
-		AC_LANG_POP([C++])
+		PKG_CHECK_MODULES([CATCH2], [catch2],, [
+			AC_LANG_PUSH([C++])
+			AC_CHECK_HEADERS([catch2/catch.hpp], [], [HEADER_NOT_FOUND_CXX([catch2/catch.hpp])])
+			AC_LANG_POP([C++])
+		])
 	fi
 fi
 
-- 
2.22.0

[libgpiod] [PATCH 3/5] bindings: cxx: Split out catch's main()

[Alexander Stein]

Compiling the source using CATCH_CONFIG_MAIN to provide main() takes
several seconds, so split it out from any library testing code, so it
really needs to be built once only.

Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
---
 bindings/cxx/tests/Makefile.am             | 3 ++-
 bindings/cxx/tests/gpiod-cxx-test-main.cpp | 2 ++
 bindings/cxx/tests/gpiod-cxx-test.cpp      | 2 --
 3 files changed, 4 insertions(+), 3 deletions(-)
 create mode 100644 bindings/cxx/tests/gpiod-cxx-test-main.cpp

diff --git a/bindings/cxx/tests/Makefile.am b/bindings/cxx/tests/Makefile.am
index d1da0d3..5800a23 100644
--- a/bindings/cxx/tests/Makefile.am
+++ b/bindings/cxx/tests/Makefile.am
@@ -15,7 +15,8 @@ AM_LDFLAGS += -pthread
 
 bin_PROGRAMS = gpiod-cxx-test
 
-gpiod_cxx_test_SOURCES =	gpiod-cxx-test.cpp \
+gpiod_cxx_test_SOURCES =	gpiod-cxx-test-main.cpp \
+				gpiod-cxx-test.cpp \
 				gpio-mockup.cpp \
 				gpio-mockup.hpp \
 				tests-chip.cpp \
diff --git a/bindings/cxx/tests/gpiod-cxx-test-main.cpp b/bindings/cxx/tests/gpiod-cxx-test-main.cpp
new file mode 100644
index 0000000..4ed06df
--- /dev/null
+++ b/bindings/cxx/tests/gpiod-cxx-test-main.cpp
@@ -0,0 +1,2 @@
+#define CATCH_CONFIG_MAIN
+#include <catch2/catch.hpp>
diff --git a/bindings/cxx/tests/gpiod-cxx-test.cpp b/bindings/cxx/tests/gpiod-cxx-test.cpp
index 236fd2d..e110a3c 100644
--- a/bindings/cxx/tests/gpiod-cxx-test.cpp
+++ b/bindings/cxx/tests/gpiod-cxx-test.cpp
@@ -5,8 +5,6 @@
  * Copyright (C) 2019 Bartosz Golaszewski <bgolaszewski@baylibre.com>
  */
 
-#define CATCH_CONFIG_MAIN
-#include <catch2/catch.hpp>
 #include <linux/version.h>
 #include <sys/utsname.h>
 #include <system_error>
-- 
2.22.0

[libgpiod] [PATCH 4/5] bindings: cxx: Fix compile errors

[Alexander Stein]

This fixes the following compile errors:
tests-event.cpp:152:3: error: cannot declare reference to
'class std::system_error&', which is not a typedef or a template type
argument
  152 |   REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);

Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
---
 bindings/cxx/tests/tests-chip.cpp  |  8 ++++----
 bindings/cxx/tests/tests-event.cpp |  4 ++--
 bindings/cxx/tests/tests-line.cpp  | 16 ++++++++--------
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
index 11c2d4c..c9eb8e5 100644
--- a/bindings/cxx/tests/tests-chip.cpp
+++ b/bindings/cxx/tests/tests-chip.cpp
@@ -107,7 +107,7 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
 
 	SECTION("using uninitialized chip throws logic_error")
 	{
-		REQUIRE_THROWS_AS(chip.name(), ::std::logic_error&);
+		REQUIRE_THROWS_AS(chip.name(), ::std::logic_error);
 	}
 }
 
@@ -139,7 +139,7 @@ TEST_CASE("GPIO chip can be opened with the open() method with implicit lookup",
 
 TEST_CASE("Trying to open a nonexistent chip throws system_error", "[chip]")
 {
-	REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error&);
+	REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error);
 }
 
 TEST_CASE("Chip object can be reset", "[chip]")
@@ -244,12 +244,12 @@ TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chi
 
 	SECTION("invalid offset (single line)")
 	{
-		REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range&);
+		REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range);
 	}
 
 	SECTION("invalid offset (multiple lines)")
 	{
-		REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range&);
+		REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range);
 	}
 
 	SECTION("line not found by name")
diff --git a/bindings/cxx/tests/tests-event.cpp b/bindings/cxx/tests/tests-event.cpp
index b34347f..b41cf7e 100644
--- a/bindings/cxx/tests/tests-event.cpp
+++ b/bindings/cxx/tests/tests-event.cpp
@@ -149,7 +149,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
 
 	SECTION("error if not requested")
 	{
-		REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
+		REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
 	}
 
 	SECTION("error if requested for values")
@@ -157,7 +157,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
 		config.request_type = ::gpiod::line_request::DIRECTION_INPUT;
 
 		line.request(config);
-		REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
+		REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
 	}
 }
 
diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
index e827e60..08ff1e8 100644
--- a/bindings/cxx/tests/tests-line.cpp
+++ b/bindings/cxx/tests/tests-line.cpp
@@ -122,7 +122,7 @@ TEST_CASE("Line bulk object works correctly", "[line][bulk]")
 	{
 		auto lines = chip.get_all_lines();
 
-		REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range&);
+		REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range);
 	}
 }
 
@@ -242,7 +242,7 @@ TEST_CASE("Exported line can be released", "[line]")
 	line.release();
 
 	REQUIRE_FALSE(line.is_requested());
-	REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
+	REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
 }
 
 TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
@@ -256,7 +256,7 @@ TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
 
 	SECTION("using uninitialized line throws logic_error")
 	{
-		REQUIRE_THROWS_AS(line.name(), ::std::logic_error&);
+		REQUIRE_THROWS_AS(line.name(), ::std::logic_error);
 	}
 }
 
@@ -271,7 +271,7 @@ TEST_CASE("Uninitialized GPIO line_bulk behaves correctly", "[line][bulk]")
 
 	SECTION("using uninitialized line_bulk throws logic_error")
 	{
-		REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error&);
+		REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error);
 	}
 }
 
@@ -289,7 +289,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
 		auto line = chip.get_line(3);
 
 		REQUIRE_NOTHROW(line.request(config));
-		REQUIRE_THROWS_AS(line.request(config), ::std::system_error&);
+		REQUIRE_THROWS_AS(line.request(config), ::std::system_error);
 	}
 
 	SECTION("request the same line twice in line_bulk")
@@ -300,7 +300,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
 		 */
 		auto lines = chip.get_lines({ 2, 3, 4, 4 });
 
-		REQUIRE_THROWS_AS(lines.request(config), ::std::system_error&);
+		REQUIRE_THROWS_AS(lines.request(config), ::std::system_error);
 	}
 }
 
@@ -312,12 +312,12 @@ TEST_CASE("Cannot get/set values of unrequested lines", "[line]")
 
 	SECTION("get value")
 	{
-		REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
+		REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
 	}
 
 	SECTION("set value")
 	{
-		REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error&);
+		REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error);
 	}
 }
 
-- 
2.22.0

[libgpiod] [PATCH 5/5] bindings: cxx: Workaround --success run

[Alexander Stein]

If run with --success, all expressions are evaluated and printed out.
But REQUIRE_FALSE(chip) tries to iterate over the chip resulting in this
backtrace:
#0  gpiod_chip_num_lines (chip=chip@entry=0x0) at ../../lib/core.c:235
#1  gpiod_line_iter_new (chip=0x0) at ../../lib/iter.c:140
#2  gpiod::(anonymous namespace)::make_line_iter (chip=0x0) at ../../../bindings/cxx/iter.cpp:29
#3  gpiod::line_iter::line_iter (this=0x7fffffffd690, owner=...) at ../../../bindings/cxx/iter.cpp:109
#4  Catch::rangeToString<gpiod::chip> (range=...) at /usr/include/catch2/catch.hpp:1959
[...]

Workaround by forcing catch2 to call gpiod::chip::operator bool().

Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
---
This actually looks like a flaw in the binding itself that the
gpiod::line_iter can't cope with an empty gpiod::chip.

 bindings/cxx/tests/tests-chip.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
index c9eb8e5..1c69872 100644
--- a/bindings/cxx/tests/tests-chip.cpp
+++ b/bindings/cxx/tests/tests-chip.cpp
@@ -70,7 +70,7 @@ TEST_CASE("GPIO chip can be opened with the open() method in different modes", "
 	mockup::probe_guard mockup_chips({ 8, 8, 8 });
 	::gpiod::chip chip;
 
-	REQUIRE_FALSE(chip);
+	REQUIRE_FALSE(!!chip);
 
 	SECTION("open by name")
 	{
@@ -102,7 +102,7 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
 
 	SECTION("uninitialized chip is 'false'")
 	{
-		REQUIRE_FALSE(chip);
+		REQUIRE_FALSE(!!chip);
 	}
 
 	SECTION("using uninitialized chip throws logic_error")
@@ -149,7 +149,7 @@ TEST_CASE("Chip object can be reset", "[chip]")
 	::gpiod::chip chip(mockup::instance().chip_name(0));
 	REQUIRE(chip);
 	chip.reset();
-	REQUIRE_FALSE(chip);
+	REQUIRE_FALSE(!!chip);
 }
 
 TEST_CASE("Chip info can be correctly retrieved", "[chip]")
-- 
2.22.0

Re: [libgpiod] [PATCH 4/5] bindings: cxx: Fix compile errors

[Bartosz Golaszewski]

śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
napisał(a):
>
> This fixes the following compile errors:
> tests-event.cpp:152:3: error: cannot declare reference to
> 'class std::system_error&', which is not a typedef or a template type
> argument
>   152 |   REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
>

Hi Alexander, thanks for working on this!

I'm getting the following warning when I don't use the reference:

In file included from tests-line.cpp:8:
tests-line.cpp: In function ‘void ____C_A_T_C_H____T_E_S_T____24()’:
tests-line.cpp:254:45: warning: catching polymorphic type ‘class
std::system_error’ by value [-Wcatch-value=]
  REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
                                             ^~~~~~~~~~~~
I'm also not getting any build errors with my current next or master
branch. My gcc is:

gcc (Debian 8.3.0-6) 8.3.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Bart

> Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> ---
>  bindings/cxx/tests/tests-chip.cpp  |  8 ++++----
>  bindings/cxx/tests/tests-event.cpp |  4 ++--
>  bindings/cxx/tests/tests-line.cpp  | 16 ++++++++--------
>  3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
> index 11c2d4c..c9eb8e5 100644
> --- a/bindings/cxx/tests/tests-chip.cpp
> +++ b/bindings/cxx/tests/tests-chip.cpp
> @@ -107,7 +107,7 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
>
>         SECTION("using uninitialized chip throws logic_error")
>         {
> -               REQUIRE_THROWS_AS(chip.name(), ::std::logic_error&);
> +               REQUIRE_THROWS_AS(chip.name(), ::std::logic_error);
>         }
>  }
>
> @@ -139,7 +139,7 @@ TEST_CASE("GPIO chip can be opened with the open() method with implicit lookup",
>
>  TEST_CASE("Trying to open a nonexistent chip throws system_error", "[chip]")
>  {
> -       REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error&);
> +       REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error);
>  }
>
>  TEST_CASE("Chip object can be reset", "[chip]")
> @@ -244,12 +244,12 @@ TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chi
>
>         SECTION("invalid offset (single line)")
>         {
> -               REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range&);
> +               REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range);
>         }
>
>         SECTION("invalid offset (multiple lines)")
>         {
> -               REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range&);
> +               REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range);
>         }
>
>         SECTION("line not found by name")
> diff --git a/bindings/cxx/tests/tests-event.cpp b/bindings/cxx/tests/tests-event.cpp
> index b34347f..b41cf7e 100644
> --- a/bindings/cxx/tests/tests-event.cpp
> +++ b/bindings/cxx/tests/tests-event.cpp
> @@ -149,7 +149,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
>
>         SECTION("error if not requested")
>         {
> -               REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> +               REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
>         }
>
>         SECTION("error if requested for values")
> @@ -157,7 +157,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
>                 config.request_type = ::gpiod::line_request::DIRECTION_INPUT;
>
>                 line.request(config);
> -               REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> +               REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
>         }
>  }
>
> diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
> index e827e60..08ff1e8 100644
> --- a/bindings/cxx/tests/tests-line.cpp
> +++ b/bindings/cxx/tests/tests-line.cpp
> @@ -122,7 +122,7 @@ TEST_CASE("Line bulk object works correctly", "[line][bulk]")
>         {
>                 auto lines = chip.get_all_lines();
>
> -               REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range&);
> +               REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range);
>         }
>  }
>
> @@ -242,7 +242,7 @@ TEST_CASE("Exported line can be released", "[line]")
>         line.release();
>
>         REQUIRE_FALSE(line.is_requested());
> -       REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
> +       REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
>  }
>
>  TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
> @@ -256,7 +256,7 @@ TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
>
>         SECTION("using uninitialized line throws logic_error")
>         {
> -               REQUIRE_THROWS_AS(line.name(), ::std::logic_error&);
> +               REQUIRE_THROWS_AS(line.name(), ::std::logic_error);
>         }
>  }
>
> @@ -271,7 +271,7 @@ TEST_CASE("Uninitialized GPIO line_bulk behaves correctly", "[line][bulk]")
>
>         SECTION("using uninitialized line_bulk throws logic_error")
>         {
> -               REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error&);
> +               REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error);
>         }
>  }
>
> @@ -289,7 +289,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
>                 auto line = chip.get_line(3);
>
>                 REQUIRE_NOTHROW(line.request(config));
> -               REQUIRE_THROWS_AS(line.request(config), ::std::system_error&);
> +               REQUIRE_THROWS_AS(line.request(config), ::std::system_error);
>         }
>
>         SECTION("request the same line twice in line_bulk")
> @@ -300,7 +300,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
>                  */
>                 auto lines = chip.get_lines({ 2, 3, 4, 4 });
>
> -               REQUIRE_THROWS_AS(lines.request(config), ::std::system_error&);
> +               REQUIRE_THROWS_AS(lines.request(config), ::std::system_error);
>         }
>  }
>
> @@ -312,12 +312,12 @@ TEST_CASE("Cannot get/set values of unrequested lines", "[line]")
>
>         SECTION("get value")
>         {
> -               REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
> +               REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
>         }
>
>         SECTION("set value")
>         {
> -               REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error&);
> +               REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error);
>         }
>  }
>
> --
> 2.22.0
>

Re: [libgpiod] [PATCH 5/5] bindings: cxx: Workaround --success run

[Bartosz Golaszewski]

śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
napisał(a):
>
> If run with --success, all expressions are evaluated and printed out.
> But REQUIRE_FALSE(chip) tries to iterate over the chip resulting in this
> backtrace:
> #0  gpiod_chip_num_lines (chip=chip@entry=0x0) at ../../lib/core.c:235
> #1  gpiod_line_iter_new (chip=0x0) at ../../lib/iter.c:140
> #2  gpiod::(anonymous namespace)::make_line_iter (chip=0x0) at ../../../bindings/cxx/iter.cpp:29
> #3  gpiod::line_iter::line_iter (this=0x7fffffffd690, owner=...) at ../../../bindings/cxx/iter.cpp:109
> #4  Catch::rangeToString<gpiod::chip> (range=...) at /usr/include/catch2/catch.hpp:1959
> [...]
>
> Workaround by forcing catch2 to call gpiod::chip::operator bool().
>
> Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> ---
> This actually looks like a flaw in the binding itself that the
> gpiod::line_iter can't cope with an empty gpiod::chip.
>

Do you want to submit a patch that fixes that? Otherwise I can fix it.
I think that simply throwing an exception on empty chip is enough,
right?

Bart

>  bindings/cxx/tests/tests-chip.cpp | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
> index c9eb8e5..1c69872 100644
> --- a/bindings/cxx/tests/tests-chip.cpp
> +++ b/bindings/cxx/tests/tests-chip.cpp
> @@ -70,7 +70,7 @@ TEST_CASE("GPIO chip can be opened with the open() method in different modes", "
>         mockup::probe_guard mockup_chips({ 8, 8, 8 });
>         ::gpiod::chip chip;
>
> -       REQUIRE_FALSE(chip);
> +       REQUIRE_FALSE(!!chip);
>
>         SECTION("open by name")
>         {
> @@ -102,7 +102,7 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
>
>         SECTION("uninitialized chip is 'false'")
>         {
> -               REQUIRE_FALSE(chip);
> +               REQUIRE_FALSE(!!chip);
>         }
>
>         SECTION("using uninitialized chip throws logic_error")
> @@ -149,7 +149,7 @@ TEST_CASE("Chip object can be reset", "[chip]")
>         ::gpiod::chip chip(mockup::instance().chip_name(0));
>         REQUIRE(chip);
>         chip.reset();
> -       REQUIRE_FALSE(chip);
> +       REQUIRE_FALSE(!!chip);
>  }
>
>  TEST_CASE("Chip info can be correctly retrieved", "[chip]")
> --
> 2.22.0
>

Re: [libgpiod] [PATCH 4/5] bindings: cxx: Fix compile errors

[Bartosz Golaszewski]

czw., 8 sie 2019 o 17:25 Bartosz Golaszewski <brgl@bgdev.pl> napisał(a):
>
> śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
> napisał(a):
> >
> > This fixes the following compile errors:
> > tests-event.cpp:152:3: error: cannot declare reference to
> > 'class std::system_error&', which is not a typedef or a template type
> > argument
> >   152 |   REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> >
>
> Hi Alexander, thanks for working on this!
>
> I'm getting the following warning when I don't use the reference:
>
> In file included from tests-line.cpp:8:
> tests-line.cpp: In function ‘void ____C_A_T_C_H____T_E_S_T____24()’:
> tests-line.cpp:254:45: warning: catching polymorphic type ‘class
> std::system_error’ by value [-Wcatch-value=]
>   REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
>                                              ^~~~~~~~~~~~
> I'm also not getting any build errors with my current next or master
> branch. My gcc is:
>
> gcc (Debian 8.3.0-6) 8.3.0
> Copyright (C) 2018 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions.  There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> Bart

Nevermind that - I was using Debian's packaged version which happens
to be quite old. The current version actually does fail to build. I'll
test and pick up your patches tomorrow.

Bart

>
> > Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> > ---
> >  bindings/cxx/tests/tests-chip.cpp  |  8 ++++----
> >  bindings/cxx/tests/tests-event.cpp |  4 ++--
> >  bindings/cxx/tests/tests-line.cpp  | 16 ++++++++--------
> >  3 files changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/bindings/cxx/tests/tests-chip.cpp b/bindings/cxx/tests/tests-chip.cpp
> > index 11c2d4c..c9eb8e5 100644
> > --- a/bindings/cxx/tests/tests-chip.cpp
> > +++ b/bindings/cxx/tests/tests-chip.cpp
> > @@ -107,7 +107,7 @@ TEST_CASE("Uninitialized GPIO chip behaves correctly", "[chip]")
> >
> >         SECTION("using uninitialized chip throws logic_error")
> >         {
> > -               REQUIRE_THROWS_AS(chip.name(), ::std::logic_error&);
> > +               REQUIRE_THROWS_AS(chip.name(), ::std::logic_error);
> >         }
> >  }
> >
> > @@ -139,7 +139,7 @@ TEST_CASE("GPIO chip can be opened with the open() method with implicit lookup",
> >
> >  TEST_CASE("Trying to open a nonexistent chip throws system_error", "[chip]")
> >  {
> > -       REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error&);
> > +       REQUIRE_THROWS_AS(::gpiod::chip("nonexistent-chip"), ::std::system_error);
> >  }
> >
> >  TEST_CASE("Chip object can be reset", "[chip]")
> > @@ -244,12 +244,12 @@ TEST_CASE("Errors occurring when retrieving lines are correctly reported", "[chi
> >
> >         SECTION("invalid offset (single line)")
> >         {
> > -               REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range&);
> > +               REQUIRE_THROWS_AS(chip.get_line(9), ::std::out_of_range);
> >         }
> >
> >         SECTION("invalid offset (multiple lines)")
> >         {
> > -               REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range&);
> > +               REQUIRE_THROWS_AS(chip.get_lines({ 1, 19, 4, 7 }), ::std::out_of_range);
> >         }
> >
> >         SECTION("line not found by name")
> > diff --git a/bindings/cxx/tests/tests-event.cpp b/bindings/cxx/tests/tests-event.cpp
> > index b34347f..b41cf7e 100644
> > --- a/bindings/cxx/tests/tests-event.cpp
> > +++ b/bindings/cxx/tests/tests-event.cpp
> > @@ -149,7 +149,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
> >
> >         SECTION("error if not requested")
> >         {
> > -               REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> > +               REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
> >         }
> >
> >         SECTION("error if requested for values")
> > @@ -157,7 +157,7 @@ TEST_CASE("It's possible to retrieve the event file descriptor", "[event][line]"
> >                 config.request_type = ::gpiod::line_request::DIRECTION_INPUT;
> >
> >                 line.request(config);
> > -               REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error&);
> > +               REQUIRE_THROWS_AS(line.event_get_fd(), ::std::system_error);
> >         }
> >  }
> >
> > diff --git a/bindings/cxx/tests/tests-line.cpp b/bindings/cxx/tests/tests-line.cpp
> > index e827e60..08ff1e8 100644
> > --- a/bindings/cxx/tests/tests-line.cpp
> > +++ b/bindings/cxx/tests/tests-line.cpp
> > @@ -122,7 +122,7 @@ TEST_CASE("Line bulk object works correctly", "[line][bulk]")
> >         {
> >                 auto lines = chip.get_all_lines();
> >
> > -               REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range&);
> > +               REQUIRE_THROWS_AS(lines.get(11), ::std::out_of_range);
> >         }
> >  }
> >
> > @@ -242,7 +242,7 @@ TEST_CASE("Exported line can be released", "[line]")
> >         line.release();
> >
> >         REQUIRE_FALSE(line.is_requested());
> > -       REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
> > +       REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
> >  }
> >
> >  TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
> > @@ -256,7 +256,7 @@ TEST_CASE("Uninitialized GPIO line behaves correctly", "[line]")
> >
> >         SECTION("using uninitialized line throws logic_error")
> >         {
> > -               REQUIRE_THROWS_AS(line.name(), ::std::logic_error&);
> > +               REQUIRE_THROWS_AS(line.name(), ::std::logic_error);
> >         }
> >  }
> >
> > @@ -271,7 +271,7 @@ TEST_CASE("Uninitialized GPIO line_bulk behaves correctly", "[line][bulk]")
> >
> >         SECTION("using uninitialized line_bulk throws logic_error")
> >         {
> > -               REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error&);
> > +               REQUIRE_THROWS_AS(bulk.get(0), ::std::logic_error);
> >         }
> >  }
> >
> > @@ -289,7 +289,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
> >                 auto line = chip.get_line(3);
> >
> >                 REQUIRE_NOTHROW(line.request(config));
> > -               REQUIRE_THROWS_AS(line.request(config), ::std::system_error&);
> > +               REQUIRE_THROWS_AS(line.request(config), ::std::system_error);
> >         }
> >
> >         SECTION("request the same line twice in line_bulk")
> > @@ -300,7 +300,7 @@ TEST_CASE("Cannot request the same line twice", "[line]")
> >                  */
> >                 auto lines = chip.get_lines({ 2, 3, 4, 4 });
> >
> > -               REQUIRE_THROWS_AS(lines.request(config), ::std::system_error&);
> > +               REQUIRE_THROWS_AS(lines.request(config), ::std::system_error);
> >         }
> >  }
> >
> > @@ -312,12 +312,12 @@ TEST_CASE("Cannot get/set values of unrequested lines", "[line]")
> >
> >         SECTION("get value")
> >         {
> > -               REQUIRE_THROWS_AS(line.get_value(), ::std::system_error&);
> > +               REQUIRE_THROWS_AS(line.get_value(), ::std::system_error);
> >         }
> >
> >         SECTION("set value")
> >         {
> > -               REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error&);
> > +               REQUIRE_THROWS_AS(line.set_value(1), ::std::system_error);
> >         }
> >  }
> >
> > --
> > 2.22.0
> >

Re: [libgpiod] [PATCH 5/5] bindings: cxx: Workaround --success run

[Alexander Stein]

 On Thursday, August 8, 2019, 5:27:14 PM CEST Bartosz Golaszewski wrote:
> śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
> napisał(a):
> >
> > If run with --success, all expressions are evaluated and printed out.
> > But REQUIRE_FALSE(chip) tries to iterate over the chip resulting in this
> > backtrace:
> > #0  gpiod_chip_num_lines (chip=chip@entry=0x0) at ../../lib/core.c:235
> > #1  gpiod_line_iter_new (chip=0x0) at ../../lib/iter.c:140
> > #2  gpiod::(anonymous namespace)::make_line_iter (chip=0x0) at ../../../bindings/cxx/iter.cpp:29
> > #3  gpiod::line_iter::line_iter (this=0x7fffffffd690, owner=...) at ../../../bindings/cxx/iter.cpp:109
> > #4  Catch::rangeToString<gpiod::chip> (range=...) at /usr/include/catch2/catch.hpp:1959
> > [...]
> >
> > Workaround by forcing catch2 to call gpiod::chip::operator bool().
> >
> > Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> > ---
> > This actually looks like a flaw in the binding itself that the
> > gpiod::line_iter can't cope with an empty gpiod::chip.
> >
> 
> Do you want to submit a patch that fixes that? Otherwise I can fix it.
> I think that simply throwing an exception on empty chip is enough,
> right?

Reading that backtrace today, the actual problem is gpiod_chip_num_lines deferencing the nullptr.
There are 2 possibilities:
* if gpiod_chip is NULL in gpiod_line_iter_new(), return NULLL iter as well (which will raise an exception on line iter.cpp:31)
* return an iter with num_lines = 0

Can't rate the 2nd one if this will raise other problems.

Best regards,
Alexander

Re: [libgpiod] [PATCH 5/5] bindings: cxx: Workaround --success run

[Bartosz Golaszewski]

czw., 8 sie 2019 o 20:41 Alexander Stein <alexander.stein@mailbox.org>
napisał(a):
>
>  On Thursday, August 8, 2019, 5:27:14 PM CEST Bartosz Golaszewski wrote:
> > śr., 7 sie 2019 o 21:51 Alexander Stein <alexander.stein@mailbox.org>
> > napisał(a):
> > >
> > > If run with --success, all expressions are evaluated and printed out.
> > > But REQUIRE_FALSE(chip) tries to iterate over the chip resulting in this
> > > backtrace:
> > > #0  gpiod_chip_num_lines (chip=chip@entry=0x0) at ../../lib/core.c:235
> > > #1  gpiod_line_iter_new (chip=0x0) at ../../lib/iter.c:140
> > > #2  gpiod::(anonymous namespace)::make_line_iter (chip=0x0) at ../../../bindings/cxx/iter.cpp:29
> > > #3  gpiod::line_iter::line_iter (this=0x7fffffffd690, owner=...) at ../../../bindings/cxx/iter.cpp:109
> > > #4  Catch::rangeToString<gpiod::chip> (range=...) at /usr/include/catch2/catch.hpp:1959
> > > [...]
> > >
> > > Workaround by forcing catch2 to call gpiod::chip::operator bool().
> > >
> > > Signed-off-by: Alexander Stein <alexander.stein@mailbox.org>
> > > ---
> > > This actually looks like a flaw in the binding itself that the
> > > gpiod::line_iter can't cope with an empty gpiod::chip.
> > >
> >
> > Do you want to submit a patch that fixes that? Otherwise I can fix it.
> > I think that simply throwing an exception on empty chip is enough,
> > right?
>
> Reading that backtrace today, the actual problem is gpiod_chip_num_lines deferencing the nullptr.
> There are 2 possibilities:
> * if gpiod_chip is NULL in gpiod_line_iter_new(), return NULLL iter as well (which will raise an exception on line iter.cpp:31)
> * return an iter with num_lines = 0
>
> Can't rate the 2nd one if this will raise other problems.
>
> Best regards,
> Alexander
>
>
>

Hardening of argument validation in the core library (checking of
NULL-pointers, etc.) has been on my TODO list for some time now. It
seems I'll finally have to do it.

Thanks for the patches, all applied!

Bartosz