Hi Marcel, * Marcel Holtmann <marcel@xxxxxxxxxxxx> [2010-11-10 14:23:42 +0900]: > Hi Gustavo, > > > > On Mon, Nov 08, 2010, Gustavo F. Padovan wrote: > > > > Here are some patches that try to fix the mess of reporting error to > > > > DBus inside BlueZ. It follows the oFono and ConnMan error system. > > > > > > > > The goal is to get ride of any directly call to g_dbus_create_error() > > > > inside bluez code, changing that to __btd_error_*. This patch set > > > > doesn't fix all of them yet, but is a very good start. Please review. > > > > > > > > > > > > Gustavo F. Padovan (9): > > > > Create __btd_error_invalid_args() > > > > Add __btd_error_already_exists() > > > > Add __btd_error_not_supported() > > > > Add __btd_error_not_connected() > > > > Add __btd_error_in_progress() > > > > Add __btd_error_not_available() > > > > Add __btd_error_busy() > > > > Add __btd_error_does_not_exist() > > > > Add __btd_error_not_authorized() > > > > > > The patches seem fine to me, but before pushing upstream I'd like to > > > understand the reason for prefixing these with with __btd instead of > > > btd. What's the criteria used to decide what to use and when and why is > > > __btd the correct choice for these new functions? My first guess would > > > have been that __btd is for things only accessible by the core-daemon > > > whereas btd is for functions exported to plugins, but that doesn't seem > > > to be the case with your patches since many of these __btd functions get > > > called from plugins. > > > > I just followed oFono and ConnMan on this. That is the reason and I > > didn't asked myself why have a __ in this case.. But I see your point. > > Do you think that change that to btd_error_* will fit better inside > > BlueZ? I can change that then. > > so within ConnMan and oFono we make a difference between public symbols > that are reachable from within plugins and other which are not. > > In general btd_ should be public symbols available to plugins and __btd_ > for internal symbols that are no available to plugins. > > For builtin plugins that makes no difference of course, but this is not > about internal builtin plugins. It is for protecting against external > plugins to not allow access to internal details. > > That said, bluetoothd is not linked with the case to hide certain > symbols anyway so that right now there is no real difference here. So to make that more clear, I'll change these functions to btd_ -- Gustavo F. Padovan http://profusion.mobi -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
