Hi Sebastian,
On Fri, Jun 11, 2021 at 5:32 AM Sebastian Urban <surban@xxxxxxxxxx> wrote:
>
> This checks if an outstanding call to AcquireWrite is already in
> progress. If so, the write request is placed into the queue, but
> AcquireWrite is not called again. When a response to AcquireWrite is
> received, acquire_write_reply sends all queued writes over the acquired
> socket.
>
> Making multiple simultaneous calls to AcquireWrite makes no sense,
> as this would open multiple socket pairs and only the last returned
> socket would be used for further writes.
> ---
> src/gatt-database.c | 38 ++++++++++++++++++++++++++++++--------
> 1 file changed, 30 insertions(+), 8 deletions(-)
>
> diff --git a/src/gatt-database.c b/src/gatt-database.c
> index be6dfb265..071f88583 100644
> --- a/src/gatt-database.c
> +++ b/src/gatt-database.c
> @@ -2447,6 +2447,7 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
> {
> struct pending_op *op = user_data;
> struct external_chrc *chrc;
> + struct queue *resend;
> DBusError err;
> int fd;
> uint16_t mtu;
> @@ -2488,18 +2489,35 @@ static void acquire_write_reply(DBusMessage *message, void *user_data)
>
> chrc->write_io = sock_io_new(fd, chrc);
>
> - if (sock_io_send(chrc->write_io, op->data.iov_base,
> - op->data.iov_len) < 0)
> - goto retry;
> + while ((op = queue_peek_head(chrc->pending_writes)) != NULL) {
> + if (sock_io_send(chrc->write_io, op->data.iov_base,
> + op->data.iov_len) < 0)
> + goto retry;
>
> - gatt_db_attribute_write_result(op->attrib, op->id, 0);
> + gatt_db_attribute_write_result(op->attrib, op->id, 0);
> + pending_op_free(op);
> + }
>
> return;
>
> retry:
> - send_write(op->device, op->attrib, chrc->proxy, NULL, op->id,
> - op->data.iov_base, op->data.iov_len, 0,
> - op->link_type, false, false);
> + /*
> + * send_write pushes to chrc->pending_writes, so we need a
> + * temporary queue to avoid an infinite loop.
> + */
> + resend = queue_new();
> +
> + while ((op = queue_pop_head(chrc->pending_writes)) != NULL)
> + queue_push_tail(resend, op);
> +
> + while ((op = queue_pop_head(resend)) != NULL) {
> + send_write(op->device, op->attrib, chrc->proxy, NULL, op->id,
> + op->data.iov_base, op->data.iov_len, 0,
> + op->link_type, false, false);
> + pending_op_free(op);
> + }
It might be better to have a separate function to flush the operation
on pending_writes since it just creating another pending_write_new we
could just call WriteValue directly since the original op can be
reused as we no longer free the op automatically.
> + queue_destroy(resend, NULL);
> }
>
> static void acquire_write_setup(DBusMessageIter *iter, void *user_data)
> @@ -2527,14 +2545,18 @@ static struct pending_op *acquire_write(struct external_chrc *chrc,
> uint8_t link_type)
> {
> struct pending_op *op;
> + bool acquiring = !queue_isempty(chrc->pending_writes);
>
> op = pending_write_new(device, chrc->pending_writes, attrib, id, value,
> len, 0, link_type, false, false);
>
> + if (acquiring)
> + return op;
> +
> if (g_dbus_proxy_method_call(chrc->proxy, "AcquireWrite",
> acquire_write_setup,
> acquire_write_reply,
> - op, pending_op_free))
> + op, NULL))
> return op;
>
> pending_op_free(op);
> --
> 2.25.1
>
--
Luiz Augusto von Dentz
