Re: [PATCH] Fix device_match_pattern function

[Santiago Carot-Nemesio <scarot@libresoft.es>]

Hi Luiz,

I would like to take up again this matter to discuss some estrange
behaviour that we observed when a driver is probed.

> At current moment only the
> first one is being provided when driver is probed. 

Well, the comment in update_service function is clear when Service Class
Id list is checked:

/* Extract the first element and skip the remainning */

I guess that only the first entry is valid to you to get the profile and
add it to the profile list. We don't know very well why this decision
was taken. It will very helpul for us if you can provide a brief
explanation about that. Wouldn't a driver want to know if there are more
entries in that Service Class ID List when it is probed?. For example a
driver registering for two uuids and those uuids are present in the same
Service Class ID List. Please notice that it is may happen in HDP. In
this scenario only the first entry will appear in the profile list and
then one uuid is passed to the driver when it is probed, well this is
not true at all, see below for some abnormal behaviour with repeated
uuids in the list that is provided to the driver.

Let's suppose an SDP record like this:
*Service Class ID List
	MDPSink
	MDPSource
....aditional info not relevant for this example....

In that scenario, a driver is registered for next uuids:
00001400-0000-1000-8000-00805F9B34FB -> MDP (Health device profile)
00001401-0000-1000-8000-00805F9B34FB -> MDPSource
00001402-0000-1000-8000-00805F9B34FB -> MDPSink

which is perfectly valid for a HDP driver due that no extra logic is
required to manage both roles.

In current Bluez implementation only profile MDPSink is taken in count
due that it appear first in the service class ID list. In addition it
will have affect to the uuids list provided when driver is probed wich
will have repeated uuids due to an issue matching the uuid with the
profiles list: device_match_driver function.

First time that device_match_pattern is executed with uuid
00001400-0000-1000-8000-00805F9B34FB it will provide a list with the
uuid 00001402-0000-1000-8000-00805F9B34FB wich is in the profile list
(MDPSink) and that uuid will be inserted to the uuid list.

Next iteration it will check next uuid provided in the driver:
00001401-0000-1000-8000-00805F9B34FB and device_match_pattern will
return another time a list with 00001402-0000-1000-8000-00805F9B34FB
uuid wich will be inserted another time in the for loop. Please, notice
that now we have the same uuid inserted two times in the uuid list.

Next it will check last uuid provided in the driver:
00001402-0000-1000-8000-00805F9B34FB and skip it because it is already
inserted from before iteration.

You can review the code to check it.

Of course we always can get the necessary entries from the remote SDP
record, but at least the repeated uuids can be avoided by setting a
check before to insert anything in the uuid list that it will provided
when the driver is probed:

/* match pattern driver */
match = device_match_pattern(device, *uuid, profiles);
for (; match; match = match->next) {
+	if (!g_slist_find_custom(uuids, match->data,
+				(GCompareFunc) strcasecmp)) {
		uuids = g_slist_append(uuids, match->data);
+	}
}

If you want we can prepare a patch.

Comments from you are welcome.

Thanks in advance.

Best regards.