Re: [RFC 2/2] client: Add authorized property handling to characteristic attribute

[Luiz Augusto von Dentz <luiz.dentz@gmail.com>]

Hi Grzegorz,
On Wed, May 2, 2018 at 1:09 PM Grzegorz Kolodziejczyk <
grzegorz.kolodziejczyk@codecoup.pl> wrote:

> This patch adds handling of authorized property to bluetoothctl.
> ---
>   client/gatt.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 46 insertions(+)

> diff --git a/client/gatt.c b/client/gatt.c
> index 6bf644c48..fffa86851 100644
> --- a/client/gatt.c
> +++ b/client/gatt.c
> @@ -1432,6 +1432,30 @@ static gboolean chrc_notify_acquired_exists(const
GDBusPropertyTable *property,
>          return FALSE;
>   }

> +static gboolean chrc_get_authorized(const GDBusPropertyTable *property,
> +                                       DBusMessageIter *iter, void *data)
> +{
> +       struct chrc *chrc = data;
> +       dbus_bool_t value;
> +
> +       value = chrc->authorized ? TRUE : FALSE;
> +
> +       dbus_message_iter_append_basic(iter, DBUS_TYPE_BOOLEAN, &value);
> +
> +       return TRUE;
> +}
> +
> +static gboolean chrc_get_authorized_exists(const GDBusPropertyTable
*property,
> +                                                               void
*data)
> +{
> +       struct chrc *chrc = data;
> +
> +       if (chrc->authorization_req)
> +               return TRUE;
> +
> +       return FALSE;
> +}
> +
>   static const GDBusPropertyTable chrc_properties[] = {
>          { "UUID", "s", chrc_get_uuid, NULL, NULL },
>          { "Service", "o", chrc_get_service, NULL, NULL },
> @@ -1442,6 +1466,8 @@ static const GDBusPropertyTable chrc_properties[] =
{
>                                          chrc_write_acquired_exists },
>          { "NotifyAcquired", "b", chrc_get_notify_acquired, NULL,
>                                          chrc_notify_acquired_exists },
> +       { "Authorized", "b", chrc_get_authorized, NULL,
> +
chrc_get_authorized_exists },
>          { }
>   };

> @@ -1665,6 +1691,15 @@ static void authorize_write_response(const char
*input, void *user_data)

>          chrc->authorized = true;

> +       /* Authorization check of prepare writes */
> +       if (!chrc->value_len) {
> +               reply = g_dbus_create_reply(pending_message,
DBUS_TYPE_INVALID);
> +               g_dbus_send_message(aad->conn, reply);
> +               g_free(aad);
> +
> +               return;
> +       }
> +
>          errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,

chrc->max_val_len);
>          if (errsv == -EINVAL) {
> @@ -1701,8 +1736,16 @@ static DBusMessage
*chrc_write_value(DBusConnection *conn, DBusMessage *msg,
>          DBusMessageIter iter;
>          char *str;
>          int errsv;
> +       uint16_t offset = 0;

>          dbus_message_iter_init(msg, &iter);
> +       /* Get offset only (second parameter) */
> +       dbus_message_iter_next(&iter);
> +
> +       parse_options(&iter, &offset, NULL, NULL, NULL);
> +
> +       if (chrc->authorization_req && offset == 0)
> +               chrc->authorized = false;

>          if (chrc->authorization_req && !chrc->authorized) {
>                  struct authorize_attribute_data *aad;
> @@ -1722,6 +1765,9 @@ static DBusMessage *chrc_write_value(DBusConnection
*conn, DBusMessage *msg,
>                  return NULL;
>          }

> +       /* Rewind to value parameter */
> +       dbus_message_iter_init(msg, &iter);

I would prefer to make parse_value_arg parse the offset as well so we don't
have to reinit the iter.

>          errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,

chrc->max_val_len);
>          if (errsv == -EINVAL) {
> --
> 2.13.6

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



-- 
Luiz Augusto von Dentz