treewide: rework struct gpiod_line_bulk

From: Bartosz Golaszewski <[email protected]>

bindings/cxx/line_bulk.cpp +13 -0

+struct line_bulk_iter_deleter
+{
+	void operator()(::gpiod_line_bulk_iter *iter)
+	{
+		::gpiod_line_bulk_iter_free(iter);
+	}
+};
+
+using line_bulk_iter_ptr = ::std::unique_ptr<::gpiod_line_bulk_iter,
+					     line_bulk_iter_deleter>;
+
+line_bulk_iter_ptr make_line_bulk_iter(::gpiod_line_bulk *bulk)
+{

bindings/cxx/line_bulk.cpp +0 -0

 } /* namespace */
 

bindings/cxx/line_bulk.cpp +2 -7

+	auto bulk = this->to_line_bulk();
 	::timespec ts;
 	line_bulk ret;
 	int rv;
 
-	this->to_line_bulk(::std::addressof(bulk));
-
-	::gpiod_line_bulk_init(::std::addressof(event_bulk));
-
 	ts.tv_sec = timeout.count() / 1000000000ULL;
 	ts.tv_nsec = timeout.count() % 1000000000ULL;
 
-	rv = ::gpiod_line_event_wait_bulk(::std::addressof(bulk),
-					  ::std::addressof(ts),
-					  ::std::addressof(event_bulk));
+	rv = ::gpiod_line_event_wait_bulk(bulk.get(), ::std::addressof(ts), ev_bulk.get());
 	if (rv < 0) {
 		throw ::std::system_error(errno, ::std::system_category(),
 					  "error polling for events");

bindings/cxx/line_bulk.cpp +32 -9

 }

@@ -1715,9 +1751,10 @@ static PyObject *gpiod_LineBulk_event_wait(gpiod_LineBulkObject *self,
 {
 	static char *kwlist[] = { "sec", "nsec", NULL };
 
-	struct gpiod_line_bulk bulk, ev_bulk;
-	struct gpiod_line *line, **line_ptr;
+	struct gpiod_line_bulk *bulk, *ev_bulk;
+	struct gpiod_line_bulk_iter *iter;
 	gpiod_LineObject *line_obj;
+	struct gpiod_line *line;
 	gpiod_ChipObject *owner;
 	long sec = 0, nsec = 0;
 	struct timespec ts;

@@ -1736,37 +1773,64 @@ static PyObject *gpiod_LineBulk_event_wait(gpiod_LineBulkObject *self,
 	ts.tv_sec = sec;
 	ts.tv_nsec = nsec;
 
-	gpiod_LineBulkObjToCLineBulk(self, &bulk);
+	bulk = gpiod_LineBulkObjToCLineBulk(self);
+	if (!bulk)
+		return NULL;
+
+	ev_bulk = gpiod_line_bulk_new(self->num_lines);
+	if (!ev_bulk) {
+		gpiod_line_bulk_free(bulk);
+		return NULL;
+	}
 
 	Py_BEGIN_ALLOW_THREADS;
-	rv = gpiod_line_event_wait_bulk(&bulk, &ts, &ev_bulk);
+	rv = gpiod_line_event_wait_bulk(bulk, &ts, ev_bulk);
+	gpiod_line_bulk_free(bulk);
 	Py_END_ALLOW_THREADS;
-	if (rv < 0)
+	if (rv < 0) {
+		gpiod_line_bulk_free(ev_bulk);
 		return PyErr_SetFromErrno(PyExc_OSError);
-	else if (rv == 0)
+	} else if (rv == 0) {
+		gpiod_line_bulk_free(ev_bulk);
 		Py_RETURN_NONE;
+	}
 
-	ret = PyList_New(gpiod_line_bulk_num_lines(&ev_bulk));
-	if (!ret)
+	ret = PyList_New(gpiod_line_bulk_num_lines(ev_bulk));
+	if (!ret) {
+		gpiod_line_bulk_free(ev_bulk);
 		return NULL;
+	}
 
 	owner = ((gpiod_LineObject *)(self->lines[0]))->owner;
 
+	iter = gpiod_line_bulk_iter_new(ev_bulk);
+	if (!iter) {
+		gpiod_line_bulk_free(ev_bulk);
+		return PyErr_SetFromErrno(PyExc_OSError);
+	}
+
 	i = 0;
-	gpiod_line_bulk_foreach_line(&ev_bulk, line, line_ptr) {
+	gpiod_line_bulk_iter_foreach_line(iter, line) {
 		line_obj = gpiod_MakeLineObject(owner, line);
 		if (!line_obj) {
+			gpiod_line_bulk_iter_free(iter);
+			gpiod_line_bulk_free(ev_bulk);
 			Py_DECREF(ret);
 			return NULL;
 		}

bindings/cxx/line_bulk.cpp +19 -7

+	gpiod_line_bulk_iter_free(iter);
+	gpiod_line_bulk_free(ev_bulk);
+
 	return ret;
 }
 

@@ -2241,41 +2305,59 @@ PyDoc_STRVAR(gpiod_Chip_get_all_lines_doc,
 static gpiod_LineBulkObject *
 gpiod_Chip_get_all_lines(gpiod_ChipObject *self, PyObject *Py_UNUSED(ignored))
 {
+	struct gpiod_line_bulk_iter *iter;
 	gpiod_LineBulkObject *bulk_obj;
-	struct gpiod_line_bulk bulk;
+	struct gpiod_line_bulk *bulk;
 	gpiod_LineObject *line_obj;
 	struct gpiod_line *line;
-	unsigned int offset;
+	unsigned int index = 0;
 	PyObject *list;
 	int rv;
 
 	if (gpiod_ChipIsClosed(self))
 		return NULL;
 
-	rv = gpiod_chip_get_all_lines(self->chip, &bulk);
-	if (rv)
+	bulk = gpiod_chip_get_all_lines(self->chip);
+	if (!bulk)
 		return (gpiod_LineBulkObject *)PyErr_SetFromErrno(
 							PyExc_OSError);
 
-	list = PyList_New(gpiod_line_bulk_num_lines(&bulk));
-	if (!list)
+	list = PyList_New(gpiod_line_bulk_num_lines(bulk));
+	if (!list) {
+		gpiod_line_bulk_free(bulk);
 		return NULL;
+	}
 
-	gpiod_line_bulk_foreach_line_off(&bulk, line, offset) {
+	iter = gpiod_line_bulk_iter_new(bulk);
+	if (!iter) {
+		gpiod_line_bulk_free(bulk);
+		Py_DECREF(list);
+		return (gpiod_LineBulkObject *)PyErr_SetFromErrno(
+							PyExc_OSError);
+	}

Quoted 2 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Subject should be prefixed with [libgpiod] according to the README ;).

If you replace the gpiod_line_bulk_iter_foreach_line() here with manually looping over the bulk lines then everything from A to B above can be dropped.

Quoted 7 lines · Kent Gibson

Quoted 2 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Subject should be prefixed with [libgpiod] according to the README ;).

Oh come on! :)

[snip]

Quoted 1 line

Quoted 4 lines

If you replace the gpiod_line_bulk_iter_foreach_line() here with manually looping over the bulk lines then everything from A to B above can be dropped.

Something is telling me that dedicated iterators are more elegant. But I guess we can do everything manually for now and see about that later. Fair enough, I'll drop it.

Quoted 14 lines · Bartosz Golaszewski

Quoted 7 lines · Kent Gibson

Quoted 2 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Subject should be prefixed with [libgpiod] according to the README ;).

Oh come on! :)

[snip]

Quoted 1 line

Quoted 9 lines

Quoted 4 lines

If you replace the gpiod_line_bulk_iter_foreach_line() here with manually looping over the bulk lines then everything from A to B above can be dropped.

Something is telling me that dedicated iterators are more elegant. But I guess we can do everything manually for now and see about that later. Fair enough, I'll drop it.

Good thing I'm here to let you know that little voice is wrong :).

I've got a couple of commits in my working tree[1] that apply to your patch and drop the iters as I've suggested if you'd like to compare.

Cheers,
Kent.

[1] https://github.com/warthog618/libgpiod/commits/for-v2

Quoted 31 lines · Kent Gibson

Quoted 14 lines · Bartosz Golaszewski

Quoted 7 lines · Kent Gibson

Quoted 2 lines · Bartosz Golaszewski

From: Bartosz Golaszewski <[email protected]>

Subject should be prefixed with [libgpiod] according to the README ;).

Oh come on! :)

[snip]

Quoted 1 line

Quoted 9 lines

Quoted 4 lines

If you replace the gpiod_line_bulk_iter_foreach_line() here with manually looping over the bulk lines then everything from A to B above can be dropped.

Something is telling me that dedicated iterators are more elegant. But I guess we can do everything manually for now and see about that later. Fair enough, I'll drop it.

Good thing I'm here to let you know that little voice is wrong :).

I've got a couple of commits in my working tree[1] that apply to your patch and drop the iters as I've suggested if you'd like to compare.

Quoted 4 lines

Cheers,
Kent.

[1] https://github.com/warthog618/libgpiod/commits/for-v2

Would you mind if I squashed those changes into my patch?