All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bartosz Golaszewski <brgl@bgdev.pl>
To: Kent Gibson <warthog618@gmail.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Jack Winch <sunt.un.morcov@gmail.com>,
	Helmut Grohne <helmut.grohne@intenta.de>
Cc: linux-gpio@vger.kernel.org, Bartosz Golaszewski <brgl@bgdev.pl>
Subject: [libgpiod][PATCH 2/4] API: drop "peek" functions
Date: Mon, 21 Jun 2021 21:18:28 +0200	[thread overview]
Message-ID: <20210621191830.10628-3-brgl@bgdev.pl> (raw)
In-Reply-To: <20210621191830.10628-1-brgl@bgdev.pl>

When I took the new API for a ride writing the C++ bindings, it turned
out that Kent was right all along and the split into get and peek
functions isn't really useful, while copy functions operating on copied
objects are what we need. This removes the peek functions in favor of
"get" functions everywhere and simply documents the lifetime of each
object returned by said get functions. It also adds copy functions for
objects where it makes sense - line_info and edge_event as they can
be stored in other objects (info_event and edge_event_buffer
respectively) and the user may want to decouple their lifetime from
the parent.

This is done because in real-life situations, the user of the C API
will most likely only access the edge_event to inspect its properties
while it's still stored in the buffer and only on rare occasions will
there be a need for a deep copy.

For C++ bindings this is useful because we want to make it possible
to copy the edge_event object but also not require new allocations
for every received object for better performance.

Signed-off-by: Bartosz Golaszewski <brgl@bgdev.pl>
---
 include/gpiod.h  | 51 +++++++++++++++++-------------------------------
 lib/edge-event.c | 34 ++++++++++++--------------------
 lib/info-event.c |  8 +-------
 lib/internal.h   |  1 -
 lib/line-info.c  | 24 +++++++++++------------
 tools/gpiomon.c  |  4 ++--
 6 files changed, 46 insertions(+), 76 deletions(-)

diff --git a/include/gpiod.h b/include/gpiod.h
index f3d20d4..3671c2d 100644
--- a/include/gpiod.h
+++ b/include/gpiod.h
@@ -38,11 +38,6 @@ extern "C" {
  * users pass valid pointers to objects as arguments. An exception to this rule
  * are the functions that free/close/release resources - which work when passed
  * a NULL-pointer as argument. Other exceptions are documented.
- *
- * <p>When a function returns a pointer to an opaque, libgpiod-specific object,
- * the caller is tasked with its lifetime management except where explicitly
- * documented. Returned const char * pointers must not be freed by the caller
- * as they are managed by the containing objects.
  */
 
 struct gpiod_chip;
@@ -249,6 +244,14 @@ enum {
  */
 void gpiod_line_info_free(struct gpiod_line_info *info);
 
+/**
+ * @brief Copy the line info object.
+ * @param info Line info to copy.
+ * @return Copy of the line info or NULL on error. The returned object must
+ *         be freed by the caller using :gpiod_line_info_free.
+ */
+struct gpiod_line_info *gpiod_line_info_copy(struct gpiod_line_info *info);
+
 /**
  * @brief Get the hardware offset of the line.
  * @param info GPIO line info object.
@@ -397,17 +400,6 @@ uint64_t gpiod_info_event_get_timestamp(struct gpiod_info_event *event);
  *         the caller.
  */
 struct gpiod_line_info *
-gpiod_info_event_peek_line_info(struct gpiod_info_event *event);
-
-/**
- * @brief Get a copy of the line-info object associated with this event.
- * @param event Line info event object.
- * @return Returns a copy of the line-info object associated with this event or
- *         NULL on error. The lifetime of the returned object must be managed
- *         by the caller and the line-info object must be freed using
- *         ::gpiod_line_info_free.
- */
-struct gpiod_line_info *
 gpiod_info_event_get_line_info(struct gpiod_info_event *event);
 
 /**
@@ -936,11 +928,17 @@ enum {
 /**
  * @brief Free the edge event object.
  * @param event Edge event object to free.
- * @note Must not be used with events stored in the buffer and retrieved using
- *       ::gpiod_edge_event_buffer_peek_event.
  */
 void gpiod_edge_event_free(struct gpiod_edge_event *event);
 
+/**
+ * @brief Copy the edge event object.
+ * @param event Edge event to copy.
+ * @return Copy of the edge event or NULL on error. The returned object must
+ *         be freed by the caller using :gpiod_edge_event_free.
+ */
+struct gpiod_edge_event *gpiod_edge_event_copy(struct gpiod_edge_event *event);
+
 /**
  * @brief Get the event type.
  * @param event GPIO edge event.
@@ -1000,21 +998,8 @@ void gpiod_edge_event_buffer_free(struct gpiod_edge_event_buffer *buffer);
  * @param buffer Line event buffer.
  * @param index Index of the event in the buffer.
  * @return Pointer to an event stored in the buffer. The lifetime of this
- *         event is tied to the buffer object. Users must not call event
- *         reference counting functions on pointers retrieved using this
- *         function.
- */
-struct gpiod_edge_event *
-gpiod_edge_event_buffer_peek_event(struct gpiod_edge_event_buffer *buffer,
-				   unsigned long index);
-
-/**
- * @brief Get a copy of an edge event.
- * @param buffer Line event buffer.
- * @param index Index of the event in the buffer.
- * @return Returns a copy of the edge event stored in this buffer. The event's
- *         lifetime is managed by the caller. The event must be released using
- *         ::gpiod_edge_event_free.
+ *         event is tied to the buffer object. Users must not free the event
+ *         returned by this function.
  */
 struct gpiod_edge_event *
 gpiod_edge_event_buffer_get_event(struct gpiod_edge_event_buffer *buffer,
diff --git a/lib/edge-event.c b/lib/edge-event.c
index f8aaa33..4aece4b 100644
--- a/lib/edge-event.c
+++ b/lib/edge-event.c
@@ -35,6 +35,18 @@ GPIOD_API void gpiod_edge_event_free(struct gpiod_edge_event *event)
 	free(event);
 }
 
+GPIOD_API struct gpiod_edge_event *
+gpiod_edge_event_copy(struct gpiod_edge_event *event)
+{
+	struct gpiod_edge_event *copy;
+
+	copy = malloc(sizeof(*event));
+	if (!copy)
+		return NULL;
+
+	return copy;
+}
+
 GPIOD_API int gpiod_edge_event_get_event_type(struct gpiod_edge_event *event)
 {
 	return event->event_type;
@@ -108,36 +120,16 @@ gpiod_edge_event_buffer_free(struct gpiod_edge_event_buffer *buffer)
 	free(buffer);
 }
 
-GPIOD_API struct gpiod_edge_event *
-gpiod_edge_event_buffer_peek_event(struct gpiod_edge_event_buffer *buffer,
-				   unsigned long index)
-{
-	if (index >= buffer->num_events) {
-		errno = EINVAL;
-		return NULL;
-	}
-
-	return &buffer->events[index];
-}
-
 GPIOD_API struct gpiod_edge_event *
 gpiod_edge_event_buffer_get_event(struct gpiod_edge_event_buffer *buffer,
 				   unsigned long index)
 {
-	struct gpiod_edge_event *event;
-
 	if (index >= buffer->num_events) {
 		errno = EINVAL;
 		return NULL;
 	}
 
-	event = malloc(sizeof(*event));
-	if (!event)
-		return NULL;
-
-	memcpy(event, &buffer->events[index], sizeof(*event));
-
-	return event;
+	return &buffer->events[index];
 }
 
 GPIOD_API unsigned int
diff --git a/lib/info-event.c b/lib/info-event.c
index 3c55d36..db01db1 100644
--- a/lib/info-event.c
+++ b/lib/info-event.c
@@ -74,16 +74,10 @@ gpiod_info_event_get_timestamp(struct gpiod_info_event *event)
 	return event->timestamp;
 }
 
-GPIOD_API struct gpiod_line_info *
-gpiod_info_event_peek_line_info(struct gpiod_info_event *event)
-{
-	return event->info;
-}
-
 GPIOD_API struct gpiod_line_info *
 gpiod_info_event_get_line_info(struct gpiod_info_event *event)
 {
-	return gpiod_line_info_copy(event->info);
+	return event->info;
 }
 
 struct gpiod_info_event *gpiod_info_event_read_fd(int fd)
diff --git a/lib/internal.h b/lib/internal.h
index 225e2b0..27045fb 100644
--- a/lib/internal.h
+++ b/lib/internal.h
@@ -16,7 +16,6 @@
 
 struct gpiod_line_info *
 gpiod_line_info_from_kernel(struct gpio_v2_line_info *infobuf);
-struct gpiod_line_info *gpiod_line_info_copy(struct gpiod_line_info *info);
 int gpiod_request_config_to_kernel(struct gpiod_request_config *config,
 				   struct gpio_v2_line_request *reqbuf);
 int gpiod_line_config_to_kernel(struct gpiod_line_config *config,
diff --git a/lib/line-info.c b/lib/line-info.c
index a139f62..aed8bee 100644
--- a/lib/line-info.c
+++ b/lib/line-info.c
@@ -29,6 +29,18 @@ GPIOD_API void gpiod_line_info_free(struct gpiod_line_info *info)
 	free(info);
 }
 
+GPIOD_API struct gpiod_line_info *
+gpiod_line_info_copy(struct gpiod_line_info *info)
+{
+	struct gpiod_line_info *copy;
+
+	copy = malloc(sizeof(*info));
+	if (!copy)
+		return NULL;
+
+	return copy;
+}
+
 GPIOD_API unsigned int gpiod_line_info_get_offset(struct gpiod_line_info *info)
 {
 	return info->offset;
@@ -153,15 +165,3 @@ gpiod_line_info_from_kernel(struct gpio_v2_line_info *infobuf)
 
 	return info;
 }
-
-struct gpiod_line_info *gpiod_line_info_copy(struct gpiod_line_info *info)
-{
-	struct gpiod_line_info *new_info;
-
-	new_info = malloc(sizeof(*new_info));
-	if (!new_info)
-		return NULL;
-
-	memcpy(new_info, info, sizeof(*new_info));
-	return new_info;
-}
diff --git a/tools/gpiomon.c b/tools/gpiomon.c
index 8f07596..2b5966c 100644
--- a/tools/gpiomon.c
+++ b/tools/gpiomon.c
@@ -291,8 +291,8 @@ int main(int argc, char **argv)
 			die_perror("error reading line events");
 
 		for (i = 0; i < ret; i++) {
-			event = gpiod_edge_event_buffer_peek_event(event_buffer,
-								   i);
+			event = gpiod_edge_event_buffer_get_event(event_buffer,
+								  i);
 			if (!event)
 				die_perror("unable to retrieve the event from the buffer");
 
-- 
2.30.1


  parent reply	other threads:[~2021-06-21 19:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-21 19:18 [libgpiod][PATCH 0/4] libgpiod v2: C++ bindings Bartosz Golaszewski
2021-06-21 19:18 ` [libgpiod][PATCH 1/4] events: hide the *_read_fd() symbols from the API header Bartosz Golaszewski
2021-06-21 19:18 ` Bartosz Golaszewski [this message]
2021-06-21 19:18 ` [libgpiod][PATCH 3/4] API: add an AS_IS value for bias setting Bartosz Golaszewski
2021-06-21 19:18 ` [libgpiod][PATCH 4/4] bindings: cxx: implement C++ bindings for libgpiod v2.0 Bartosz Golaszewski
2021-06-27  8:47   ` Kent Gibson
2021-06-28 11:30     ` Andy Shevchenko
2021-06-28 11:34       ` Kent Gibson
2021-06-28 11:53         ` Andy Shevchenko
2021-06-28 14:25           ` Kent Gibson
2021-07-02  8:50     ` Bartosz Golaszewski
2021-07-02 11:38       ` Kent Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210621191830.10628-3-brgl@bgdev.pl \
    --to=brgl@bgdev.pl \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=helmut.grohne@intenta.de \
    --cc=linus.walleij@linaro.org \
    --cc=linux-gpio@vger.kernel.org \
    --cc=sunt.un.morcov@gmail.com \
    --cc=warthog618@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.