Re: [PATCH v4 1/3] shared/gatt-server: Add support for long write

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Łukasz,

On Thu, Mar 31, 2016 at 12:01 AM, Łukasz Rymanowski
<lukasz.rymanowski@xxxxxxxxxxx> wrote:
> With this patch long write and nested long write reliable is supported.
> GATT server is responsible now to do aggregation of prep write data
> for long write session.
> Note: We consider long write as the consequtive prepare writes with
> continues offsets.
>
> E.g. 1
>
> prep_write: handle 1, offset 0, value_len 10
> prep_write: handle 1, offset 10, value_len 10
> prep_write: handle 2, offset 0, value_len 10
> prep_write: handle 2, offset 10, value_len 10
>
> Will result with following calles to app:
>
> exec_write: handle 1: offset 0, value_len 20
> exec_write: handle 2: offset 0, value_len 20
>
> E.g. 2
>
> prep_write: handle 1, offset 0, value_len 10
> prep_write: handle 1, offset 2, value_len 5
> prep_write: handle 2, offset 0, value_len 10
> prep_write: handle 2, offset 4, value_len 5
>
> Will result with following calles to app:
>
> exec_write: handle 1: offset 0, value_len 10
> exec_write: handle 1: offset 2, value_len 5
> exec_write: handle 2: offset 0, value_len 10
> exec_write: handle 2: offset 4, value_len 5
>
> E.g. 3
> prep_write: handle 1, offset 0, value_len 10
> prep_write: handle 1, offset 5, value_len 5
> prep_write: handle 1, offset 10, value_len 6
>
> will result with following calles to app:
>
> exec_write: handle 1, offset 0, value 10
> exec_write: handle 1, offset 5, value 11
> ---
>  src/shared/gatt-server.c | 79 +++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 65 insertions(+), 14 deletions(-)
>
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index c41273a..ae079b1 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -1088,6 +1088,56 @@ error:
>         bt_att_send_error_rsp(server->att, opcode, 0, ecode);
>  }
>
> +static bool create_and_store_prep_data(struct bt_gatt_server *server,
> +                                               uint16_t handle, uint16_t offset,
> +                                               uint16_t length, uint8_t *value)
> +{
> +       struct prep_write_data *prep_data;
> +
> +       prep_data = new0(struct prep_write_data, 1);
> +       prep_data->length = length;
> +       if (prep_data->length) {
> +               prep_data->value = malloc(prep_data->length);
> +               if (!prep_data->value) {
> +                       return false;
> +               }
> +
> +               memcpy(prep_data->value, value, prep_data->length);
> +       }
> +
> +       prep_data->server = server;
> +       prep_data->handle = handle;
> +       prep_data->offset = offset;
> +
> +       queue_push_tail(server->prep_queue, prep_data);
> +
> +       return true;
> +}
> +
> +static bool make_aggregation_of_long_write_data(struct bt_gatt_server *server,
> +                                       struct prep_write_data *prep_data,
> +                                       uint16_t handle, uint16_t length,
> +                                       uint8_t *value)
> +{
> +       uint8_t *buf;
> +       uint16_t new_len;
> +
> +       new_len = prep_data->length + length;
> +
> +       buf = malloc(new_len);
> +       if (!buf)
> +               return false;
> +
> +       memcpy(buf, prep_data->value, prep_data->length);
> +       memcpy(buf + prep_data->length, value, length);

I think we could use realloc here instead of copying like this.

> +
> +       free(prep_data->value);
> +       prep_data->value = buf;
> +       prep_data->length = new_len;
> +
> +       return true;
> +}
> +
>  static void prep_write_cb(uint8_t opcode, const void *pdu,
>                                         uint16_t length, void *user_data)
>  {
> @@ -1097,6 +1147,7 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
>         uint16_t offset;
>         struct gatt_db_attribute *attr;
>         uint8_t ecode;
> +       bool success;
>
>         if (length < 4) {
>                 ecode = BT_ATT_ERROR_INVALID_PDU;
> @@ -1126,22 +1177,22 @@ static void prep_write_cb(uint8_t opcode, const void *pdu,
>         if (ecode)
>                 goto error;
>
> -       prep_data = new0(struct prep_write_data, 1);
> -       prep_data->length = length - 4;
> -       if (prep_data->length) {
> -               prep_data->value = malloc(prep_data->length);
> -               if (!prep_data->value) {
> -                       ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> -                       goto error;
> -               }
> -       }
> +       /* Now lets check if prep write is a continuation of long write */
> +       prep_data = queue_peek_tail(server->prep_queue);
>
> -       prep_data->server = server;
> -       prep_data->handle = handle;
> -       prep_data->offset = offset;
> -       memcpy(prep_data->value, pdu + 4, prep_data->length);
> +       if (prep_data && (prep_data->handle == handle) &&
> +                       (offset == prep_data->length + prep_data->offset))
> +               success = make_aggregation_of_long_write_data(server, prep_data,
> +                                                       handle, length - 4,
> +                                                       &((uint8_t *) pdu)[4]);
> +       else
> +               success = create_and_store_prep_data(server, handle, offset,
> +                                       length - 4, &((uint8_t *) pdu)[4]);

I would prefer if this would actually be done in a separate function,
store_prep_data which should be able to detect if the data shall be
appended or if a new item should be created.

>
> -       queue_push_tail(server->prep_queue, prep_data);
> +       if (!success) {
> +               ecode = BT_ATT_ERROR_INSUFFICIENT_RESOURCES;
> +               goto error;
> +       }
>
>         bt_att_send(server->att, BT_ATT_OP_PREP_WRITE_RSP, pdu, length, NULL,
>                                                                 NULL, NULL);
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Luiz Augusto von Dentz
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux