On Tue, 2013-01-15 at 08:15 +0100, Mikel Astiz wrote: > 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. Well, it's probably not important, but it's certainly at least desirable to catch bluez bugs early. > 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. I'd like to get them removed, but I don't care very much. So if you don't want to remove them, I can live with that. > 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/) Yep, I was only referring to the points in the last mail. -- Tanu
