Hi Tanu,
On Mon, Apr 29, 2013 at 4:38 PM, Tanu Kaskinen <tanu.kaskinen at intel.com> wrote:
> On Mon, 2013-04-29 at 16:20 +0200, Mikel Astiz wrote:
>> On Mon, Apr 29, 2013 at 4:01 PM, Tanu Kaskinen <tanu.kaskinen at intel.com> wrote:
>> > On Fri, 2013-04-26 at 12:30 -0300, jprvita at gmail.com wrote:
>> >> src/modules/bluetooth/module-bluetooth-device.c | 1 +
>> >> 1 file changed, 1 insertion(+)
>> >>
>> >> diff --git a/src/modules/bluetooth/module-bluetooth-device.c b/src/modules/bluetooth/module-bluetooth-device.c
>> >> index c877df2..8695c80 100644
>> >> --- a/src/modules/bluetooth/module-bluetooth-device.c
>> >> +++ b/src/modules/bluetooth/module-bluetooth-device.c
>> >> @@ -2243,6 +2243,7 @@ static int add_card(struct userdata *u) {
>> >> pa_proplist_sets(data.proplist, "bluez.path", device->path);
>> >> pa_proplist_setf(data.proplist, "bluez.class", "0x%06x", (unsigned) device->class);
>> >> pa_proplist_sets(data.proplist, "bluez.name", device->name);
>> >> + pa_proplist_sets(data.proplist, "bluez.alias", device->alias);
>> >
>> > device->alias is not guaranteed to be non-NULL, so this code can crash.
>> > If the Alias property is non-optional in Bluez, bluetooth-util should
>> > ensure that it is indeed always set.
>>
>> BlueZ 4 API guarantees the property will be present, and therefore
>> device->alias should always be set. The obvious exception is BlueZ
>> misbehaving, but this condition might hold true for device->name as
>> well, which we're not checking either.
>>
>> I don't think there's any other scenario where the alias could be NULL
>> and the name be set outside bluetooth-util.
>>
>> So while your point is valid, I suggest we fix this in a later patch,
>> in a similar way that the BlueZ 5 patchset does (set device_info_valid
>> to -1 if any non-optional property is missing).
>
> Ah, so this is fixed at least for BlueZ 5. Will the same property
> parsing code be used for BlueZ 4? If not, does somebody promise to write
> the fix for BlueZ 4?
The codepath will be completely different for BlueZ 4 and 5, due to
the fundamental difference in how objects and properties are gathered
with or without the ObjectManager interface.
Fixing this for BlueZ 4 is desirable but not critical IMO, since it's
very unlikely that we hit this issue (based on how BlueZ 4 implements
GetProperties()). Besides, the fix seems non-trivial and there's
always the possibility to introduce regressions.
Cheers,
Mikel
