Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <f92e2c6b4b9981405a035f957a04290133453c9a.camel@hadess.net>
Subject: Re: [PATCH BlueZ v5 6/7] adapter: Add a public function to find a
 device by path
From:   Bastien Nocera <hadess@hadess.net>
To:     Sonny Sasaka <sonnysasaka@chromium.org>,
        linux-bluetooth@vger.kernel.org
Cc:     Miao-chen Chou <mcchou@chromium.org>
Date:   Tue, 01 Dec 2020 17:27:52 +0100
In-Reply-To: <20201130215602.386545-6-sonnysasaka@chromium.org>
References: <20201130215602.386545-1-sonnysasaka@chromium.org>
         <20201130215602.386545-6-sonnysasaka@chromium.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.38.1 (3.38.1-1.fc33) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Mon, 2020-11-30 at 13:56 -0800, Sonny Sasaka wrote:
> The public function is motivated by the Battery Provider API code
> which
> needs to probe whether a device exists.
> 
> Reviewed-by: Miao-chen Chou <mcchou@chromium.org>
> 
> ---
>  src/adapter.c | 33 ++++++++++++++++++++++++---------
>  src/adapter.h |  2 ++
>  2 files changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/src/adapter.c b/src/adapter.c
> index 56d0c6eaa..03d9d29e9 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -872,6 +872,30 @@ struct btd_device
> *btd_adapter_find_device(struct btd_adapter *adapter,
>         return device;
>  }
>  
> +static int device_path_cmp(gconstpointer a, gconstpointer b)
> +{
> +       const struct btd_device *device = a;
> +       const char *path = b;
> +       const char *dev_path = device_get_path(device);
> +
> +       return strcasecmp(dev_path, path);
> +}
> +
> +struct btd_device *btd_adapter_find_device_by_path(struct
> btd_adapter *adapter,
> +                                                  const char *path)
> +{
> +       GSList *list;
> +
> +       if (!adapter)
> +               return NULL;
> +
> +       list = g_slist_find_custom(adapter->devices, path,
> device_path_cmp);
> +       if (!list)
> +               return NULL;
> +
> +       return list->data;
> +}
> +
>  static void uuid_to_uuid128(uuid_t *uuid128, const uuid_t *uuid)
>  {
>         if (uuid->type == SDP_UUID16)
> @@ -3192,15 +3216,6 @@ static gboolean property_get_roles(const
> GDBusPropertyTable *property,
>         return TRUE;
>  }
>  
> -static int device_path_cmp(gconstpointer a, gconstpointer b)
> -{
> -       const struct btd_device *device = a;
> -       const char *path = b;
> -       const char *dev_path = device_get_path(device);
> -
> -       return strcasecmp(dev_path, path);
> -}

You should move the function in a separate patch, ideally. This is
helpful to show that the function was not modified in any way.

>  static DBusMessage *remove_device(DBusConnection *conn,
>                                         DBusMessage *msg, void
> *user_data)
>  {
> diff --git a/src/adapter.h b/src/adapter.h
> index e5750a37b..60b5e3bcc 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -83,6 +83,8 @@ sdp_list_t *btd_adapter_get_services(struct
> btd_adapter *adapter);
>  struct btd_device *btd_adapter_find_device(struct btd_adapter
> *adapter,
>                                                         const
> bdaddr_t *dst,
>                                                         uint8_t
> dst_type);
> +struct btd_device *btd_adapter_find_device_by_path(struct
> btd_adapter *adapter,
> +                                                  const char *path);
>  
>  const char *adapter_get_path(struct btd_adapter *adapter);
>  const bdaddr_t *btd_adapter_get_address(struct btd_adapter
> *adapter);