| Message ID | 20190807195132.7538-5-alexander.stein@mailbox.org |
|---|---|
| State | New |
| Headers | show |
| Series | [libgpiod,1/5] bindings: cxx: Use 'upstream' include path | expand |
ś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 >
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
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
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]")
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(-)