treewide: kill global line lookup
authorBartosz Golaszewski <bgolaszewski@baylibre.com>
Wed, 2 Dec 2020 11:10:24 +0000 (12:10 +0100)
committerBartosz Golaszewski <bgolaszewski@baylibre.com>
Mon, 14 Dec 2020 14:56:57 +0000 (15:56 +0100)
Global line lookup doesn't really work correctly because GPIO line names
are not unique. We'd have to return a list of matching lines. Also: not
all chips may be accessible by user in which case the chip iterator will
fail. We'll soon be removing chip iterators entirely so for now drop the
global line lookup and let users iterate over chips themselves.

Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
bindings/cxx/examples/gpiofindcxx.cpp
bindings/cxx/gpiod.hpp
bindings/cxx/line.cpp
bindings/cxx/tests/tests-line.cpp
bindings/python/gpiodmodule.c
bindings/python/tests/gpiod_py_test.py
include/gpiod.h
lib/helpers.c
tests/tests-line.c
tools/gpiofind.c

index aeba29df11121d4ad4f51b70e74bc851cc4f5e99..e9ab64aeb0b904e513f81690dfc5497dcf947bde 100644 (file)
@@ -19,11 +19,13 @@ int main(int argc, char **argv)
                return EXIT_FAILURE;
        }
 
-       auto ret = ::gpiod::find_line(argv[1]);
-       if (!ret.first)
-               return EXIT_FAILURE;
-
-       ::std::cout << ret.second.name() << " " << ret.first.offset() << ::std::endl;
+       for (auto& chip: ::gpiod::make_chip_iter()) {
+               auto line = chip.find_line(argv[1]);
+               if (line) {
+                       ::std::cout << line.name() << " " << line.offset() << ::std::endl;
+                       return EXIT_SUCCESS;
+               }
+       }
 
-       return EXIT_SUCCESS;
+       return EXIT_FAILURE;
 }
index 0d443b0120531afc93ddcfc72e9db6ac202093bb..b258730b08cb6a884889f0eb8ecf5a5ec765e9ad 100644 (file)
@@ -548,15 +548,6 @@ private:
        friend line_iter;
 };
 
-/**
- * @brief Find a GPIO line by name. Search all GPIO chips present on the system.
- * @param name Name of the line.
- * @return Returns a <line, chip> pair where line is the line with given name
- *         and chip is the line's owner. Both objects are empty if the line was
- *         not found.
- */
-GPIOD_API ::std::pair<line, chip> find_line(const ::std::string& name);
-
 /**
  * @brief Describes a single GPIO line event.
  */
index 5589875bfcd3849bc33c87813977dbd38a583961..54382e2283b470ae5fe957a8edeb7b80d59fea57 100644 (file)
@@ -343,19 +343,4 @@ line::chip_guard::chip_guard(const line& line)
        
 }
 
-::std::pair<line, chip> find_line(const ::std::string& name)
-{
-       ::std::pair<line, chip> ret;
-
-       for (auto& it: make_chip_iter()) {
-               ret.first = it.find_line(name);
-               if (ret.first) {
-                       ret.second = it;
-                       break;
-               }
-       }
-
-       return ret;
-}
-
 } /* namespace gpiod */
index 9841bea684274f43611401c51389197f94e8d82b..a5499ac0c4553b6b5bbb1c869108ccb34c891242 100644 (file)
@@ -18,25 +18,6 @@ const ::std::string consumer = "line-test";
 
 } /* namespace */
 
-TEST_CASE("Global find_line() function works", "[line]")
-{
-       mockup::probe_guard mockup_chips({ 8, 8, 8, 8, 8 }, mockup::FLAG_NAMED_LINES);
-
-       SECTION("line found")
-       {
-               auto ret = ::gpiod::find_line("gpio-mockup-C-5");
-               REQUIRE(ret.first.offset() == 5);
-               REQUIRE(ret.first.name() == "gpio-mockup-C-5");
-               REQUIRE(ret.second.label() == "gpio-mockup-C");
-       }
-
-       SECTION("line not found")
-       {
-               auto ret = ::gpiod::find_line("nonexistent-line");
-               REQUIRE_FALSE(ret.first);
-       }
-}
-
 TEST_CASE("Line information can be correctly retrieved", "[line]")
 {
        mockup::probe_guard mockup_chips({ 8 }, mockup::FLAG_NAMED_LINES);
index 17a58b12e61e5a8c31095943a601806b5c20f62e..bcaae926324d8bc35f66527c30fbf3bf2581dbd3 100644 (file)
@@ -2671,80 +2671,6 @@ static PyTypeObject gpiod_LineIterType = {
        .tp_iternext = (iternextfunc)gpiod_LineIter_next,
 };
 
-PyDoc_STRVAR(gpiod_Module_find_line_doc,
-"find_line(name) -> gpiod.Line object or None\n"
-"\n"
-"Lookup a GPIO line by name. Search all gpiochips. Returns a gpiod.Line\n"
-"or None if a line with given name doesn't exist in the system.\n"
-"\n"
-"NOTE: the gpiod.Chip object owning the returned line must be closed\n"
-"by the caller.\n"
-"\n"
-"  name\n"
-"    Name of the line to find (string).");
-
-static gpiod_LineObject *gpiod_Module_find_line(PyObject *Py_UNUSED(self),
-                                               PyObject *args)
-{
-       gpiod_LineObject *line_obj;
-       gpiod_ChipObject *chip_obj;
-       struct gpiod_chip *chip;
-       struct gpiod_line *line;
-       const char *name;
-       int rv;
-
-       rv = PyArg_ParseTuple(args, "s", &name);
-       if (!rv)
-               return NULL;
-
-       Py_BEGIN_ALLOW_THREADS;
-       line = gpiod_line_find(name);
-       Py_END_ALLOW_THREADS;
-       if (!line) {
-               if (errno == ENOENT) {
-                       Py_INCREF(Py_None);
-                       return (gpiod_LineObject *)Py_None;
-               }
-
-               return (gpiod_LineObject *)PyErr_SetFromErrno(PyExc_OSError);
-       }
-
-       chip = gpiod_line_get_chip(line);
-
-       chip_obj = PyObject_New(gpiod_ChipObject, &gpiod_ChipType);
-       if (!chip_obj) {
-               gpiod_chip_close(chip);
-               return NULL;
-       }
-
-       chip_obj->chip = chip;
-
-       line_obj = gpiod_MakeLineObject(chip_obj, line);
-       if (!line_obj)
-               return NULL;
-
-       /*
-        * PyObject_New() set the reference count for the chip object at 1 and
-        * the call to gpiod_MakeLineObject() increased it to 2. However when
-        * we return the object to the line object to the python interpreter,
-        * there'll be only a single reference holder to the chip - the line
-        * object itself. Decrease the chip reference here manually.
-        */
-       Py_DECREF(line_obj->owner);
-
-       return line_obj;
-}
-
-static PyMethodDef gpiod_module_methods[] = {
-       {
-               .ml_name = "find_line",
-               .ml_meth = (PyCFunction)gpiod_Module_find_line,
-               .ml_flags = METH_VARARGS,
-               .ml_doc = gpiod_Module_find_line_doc,
-       },
-       { }
-};
-
 typedef struct {
        const char *name;
        PyTypeObject *typeobj;
@@ -2850,7 +2776,6 @@ static PyModuleDef gpiod_Module = {
        .m_name = "gpiod",
        .m_doc = gpiod_Module_doc,
        .m_size = -1,
-       .m_methods = gpiod_module_methods,
 };
 
 typedef struct {
index e4aaadcf742a98139292eb3cce8873bdf4166a0d..8534ce919e97ab7739ee757625db87d7629c5efe 100755 (executable)
@@ -234,24 +234,6 @@ class ChipGetLines(MockupTestCase):
 # Line test cases
 #
 
-class LineGlobalFindLine(MockupTestCase):
-
-    chip_sizes = ( 4, 8, 16 )
-    flags = gpiomockup.Mockup.FLAG_NAMED_LINES
-
-    def test_global_find_line_function(self):
-        line = gpiod.find_line('gpio-mockup-B-4')
-        self.assertNotEqual(line, None)
-        try:
-            self.assertEqual(line.owner().label(), 'gpio-mockup-B')
-            self.assertEqual(line.offset(), 4)
-        finally:
-            line.owner().close()
-
-    def test_global_find_line_function_nonexistent(self):
-        line = gpiod.find_line('nonexistent-line')
-        self.assertEqual(line, None)
-
 class LineInfo(MockupTestCase):
 
     chip_sizes = ( 8, )
index c1113bfeee33f02236392fd959588078d888cb94..5aeb7cc2f669c2dfa019524cbd5e09707c57ffd4 100644 (file)
@@ -1053,30 +1053,6 @@ int gpiod_line_event_read_fd(int fd, struct gpiod_line_event *event) GPIOD_API;
 int gpiod_line_event_read_fd_multiple(int fd, struct gpiod_line_event *events,
                                      unsigned int num_events) GPIOD_API;
 
-/**
- * @}
- *
- * @defgroup line_misc Misc line functions
- * @{
- *
- * Functions that didn't fit anywhere else.
- */
-
-/**
- * @brief Find a GPIO line by its name.
- * @param name Name of the GPIO line.
- * @return Returns the GPIO line handle if the line exists in the system or
- *         NULL if it couldn't be located or an error occurred.
- * @attention GPIO lines are not unique in the linux kernel, neither globally
- *            nor within a single chip. This function finds the first line with
- *            given name.
- *
- * If this routine succeeds, the user must manually close the GPIO chip owning
- * this line to avoid memory leaks. If the line could not be found, this
- * functions sets errno to ENOENT.
- */
-struct gpiod_line *gpiod_line_find(const char *name) GPIOD_API;
-
 /**
  * @}
  *
index 2063c3f1d2bf0ea20c8ac530b29e107991715a73..5a7373639348dd8e44e83d8e6f0acab698b99de4 100644 (file)
@@ -377,32 +377,3 @@ int gpiod_line_request_bulk_both_edges_events_flags(
        return line_event_request_type_bulk(bulk, consumer, flags,
                                        GPIOD_LINE_REQUEST_EVENT_BOTH_EDGES);
 }
-
-struct gpiod_line *gpiod_line_find(const char *name)
-{
-       struct gpiod_chip_iter *iter;
-       struct gpiod_chip *chip;
-       struct gpiod_line *line;
-
-       iter = gpiod_chip_iter_new();
-       if (!iter)
-               return NULL;
-
-       gpiod_foreach_chip(iter, chip) {
-               line = gpiod_chip_find_line(chip, name);
-               if (line) {
-                       gpiod_chip_iter_free_noclose(iter);
-                       return line;
-               }
-
-               if (errno != ENOENT)
-                       goto out;
-       }
-
-       errno = ENOENT;
-
-out:
-       gpiod_chip_iter_free(iter);
-
-       return NULL;
-}
index 235df0f6a3ba46d53c5c4d64272c67dc30c964d5..aee85fef155194dddcdea11c25d70b30c7dcc28c 100644 (file)
@@ -716,37 +716,6 @@ GPIOD_TEST_CASE(output_value_caching, 0, { 8 })
        g_assert_cmpint(gpiod_test_chip_get_value(0, 2), ==, 0);
 }
 
-GPIOD_TEST_CASE(find_good, GPIOD_TEST_FLAG_NAMED_LINES, { 16, 16, 32, 16 })
-{
-       g_autoptr(gpiod_chip_struct) chip = NULL;
-       struct gpiod_line *line;
-
-       line = gpiod_line_find("gpio-mockup-C-12");
-       g_assert_nonnull(line);
-       gpiod_test_return_if_failed();
-       chip = gpiod_line_get_chip(line);
-       g_assert_cmpint(gpiod_line_offset(line), ==, 12);
-}
-
-GPIOD_TEST_CASE(find_not_found,
-               GPIOD_TEST_FLAG_NAMED_LINES, { 16, 16, 32, 16 })
-{
-       struct gpiod_line *line;
-
-       line = gpiod_line_find("nonexistent");
-       g_assert_null(line);
-       g_assert_cmpint(errno, ==, ENOENT);
-}
-
-GPIOD_TEST_CASE(find_unnamed_lines, 0, { 16, 16, 32, 16 })
-{
-       struct gpiod_line *line;
-
-       line = gpiod_line_find("gpio-mockup-C-12");
-       g_assert_null(line);
-       g_assert_cmpint(errno, ==, ENOENT);
-}
-
 GPIOD_TEST_CASE(direction, 0, { 8 })
 {
        g_autoptr(gpiod_chip_struct) chip = NULL;
index 2138ebf0e104ab6c4811abaf31a3fd05eaa3fe53..489cf339666a9c69bfdd0fb58cb9d248886873fe 100644 (file)
@@ -34,9 +34,10 @@ static void print_help(void)
 
 int main(int argc, char **argv)
 {
-       int optc, opti, ret = EXIT_SUCCESS;
+       struct gpiod_chip_iter *iter;
        struct gpiod_chip *chip;
        struct gpiod_line *line;
+       int optc, opti;
 
        for (;;) {
                optc = getopt_long(argc, argv, shortopts, longopts, &opti);
@@ -63,19 +64,23 @@ int main(int argc, char **argv)
        if (argc != 1)
                die("exactly one GPIO line name must be specified");
 
-       line = gpiod_line_find(argv[0]);
-       if (!line) {
-               if (errno == ENOENT)
-                       return EXIT_FAILURE;
+       iter = gpiod_chip_iter_new();
+       if (!iter)
+               die_perror("unable to access GPIO chips");
 
-               die_perror("error performing the line lookup");
-       }
-
-       chip = gpiod_line_get_chip(line);
-
-       printf("%s %u\n", gpiod_chip_name(chip), gpiod_line_offset(line));
+       gpiod_foreach_chip(iter, chip) {
+               line = gpiod_chip_find_line(chip, argv[0]);
+               if (line) {
+                       printf("%s %u\n",
+                              gpiod_chip_name(chip), gpiod_line_offset(line));
+                       gpiod_chip_iter_free(iter);
+                       return EXIT_SUCCESS;
+               }
 
-       gpiod_chip_close(chip);
+               if (errno != ENOENT)
+                       die_perror("error performing the line lookup");
+       }
 
-       return ret;
+       gpiod_chip_iter_free(iter);
+       return EXIT_FAILURE;
 }