Hi Tanu, On Mon, Jan 14, 2013 at 8:28 PM, Tanu Kaskinen <tanuk at iki.fi> wrote: > On Tue, 2013-01-08 at 07:14 +0200, Tanu Kaskinen wrote: >> On Mon, 2013-01-07 at 14:14 +0100, Mikel Astiz wrote: >> > On Sun, Dec 30, 2012 at 2:21 PM, Tanu Kaskinen <tanuk at iki.fi> wrote: >> > > you only check that the name and address are set, why not other >> > > properties? >> > >> > I chose these two because of being non-optional in both BlueZ 4 and 5 >> > and are at the same time being used by PA. We could also check the >> > alias, paired and trusted properties. >> >> I checked earlier, and it seems that we don't use the alias for >> anything, so checking it is unnecessary (and we should stop parsing it >> altogether). I haven't checked the paired and trusted properties, are we >> using those for anything? >> >> An example (I don't know if it's the only one) of a property that we >> don't check but should check is uuids. > > Any comments about this? I seems that you didn't incorporate these > suggested changes in the updated patches. Not sure if you're referring to the UUID only. If that's the case, you're right, I haven't changed that since the BlueZ 4 code doesn't do it either. In any case, I could integrated it here if you consider this important: one possible approach would be to add a bool uuids_ready to struct pa_bluetooth_device, since we have to distinguish the empty-uuid-list case vs property-not-present. Regarding "alias" and "trusted", I think they don't do any harm and some other module might potentially be interested in these fields. Again, if you want to keep the codebase minimal, we can remove them trivially. Besides that, I think the rest of the changes mentioned in this thread have been incorporated: - Comment removed instead of updated - All non-optional properties checked (with the exception of UUID), and otherwise fail in parse_device_properties() - D-Bus message signature check - Style fixes - Log trace fixed (s/address/object path/) Cheers, Mikel
