(12:11:48 IST) develneubee: hey Johan , http://en.pastebin.ca/886494 . let me know what u think on this.
---
(12:16:52 IST) jhe: ok
(12:16:56 IST) jhe: couple of things
(12:17:05 IST) jhe: you should use more static declarations
(12:17:15 IST) jhe: everything that can be static should be static
(12:17:21 IST) develneubee: :) ok
(12:17:31 IST) jhe: also use more descriptive variable names
(12:17:41 IST) jhe: e.g. plugin_handle instead of handle
(12:18:13 IST) develneubee: actually its just a initial draft so i didnt pay attention to the variable names
(12:18:19 IST) develneubee: but ur right
(12:18:42 IST) jhe: don't hardcode any predefined plugin names
(12:19:15 IST) jhe: the list of available plugins is determined by the contents of /var/lib/bluetooth and the selected one is determined by the value in audio.conf
(12:19:26 IST) develneubee: then were would we get the path from ?
(12:19:47 IST) jhe: what path?
(12:20:23 IST) develneubee: how does /var/lib/bluetooth help in knowing the available plugins?
(12:20:49 IST) jhe: well, libc provides directory contents listing functions doesn't it?
(12:20:55 IST) jhe: however that wont be needed
(12:21:01 IST) develneubee: the filepath of the plugin. its used in dlopen
(12:21:12 IST) jhe: we can do the same thing as hcid does with services
(12:21:23 IST) develneubee: what dir would i want to look at ?
(12:21:29 IST) jhe: when you have a .service file in /etc/bluetooth it only containts something like Name=audio
(12:21:58 IST) jhe: in that case hcid uses /usr/lib/bluetooth/bluetoothd-service-audio
(12:22:52 IST) jhe: so we can do so that if audio.conf contains e.g. Plugin=dummy, the audio service will open /usr/lib/bluetooth/telephony-plugin-dummy.so
(12:23:26 IST) jhe: (just as an example of course -- the telephony-plugin- prefix is negotiatable)
(12:23:38 IST) develneubee: ok. i dont know how hcid does that. ill dig in hcid code.
(12:23:57 IST) jhe: you don't need to, also hcid doesn't use dlopen
(12:24:13 IST) jhe: I don't understand what the problem is. it's really simple
(12:24:17 IST) jhe: you get a string from audio.conf
(12:24:22 IST) develneubee: right
(12:24:42 IST) jhe: you concatenate it with /usr/lib/bluetooth/something-string.so
(12:24:42 IST) develneubee: how would i load the lib without dlopen ?
(12:24:48 IST) jhe: and you hand over the produced string to dlopen
(12:25:01 IST) jhe: when did I say that you wouldn't use dlopen?
(12:26:12 IST) develneubee: i am a bit confused !! you just mentioned what i have done
(12:26:32 IST) develneubee: u said that we shud use what hcid uses , instead of what i have done
(12:27:11 IST) jhe: we should use the same method of producing a full-path filename as hcid does
(12:27:17 IST) jhe: that's the only part we should do the same as hcid
(12:27:39 IST) develneubee: ohhh ok.
(12:27:52 IST) jhe: and the only part different compared to what you have done is not to hardcode any pre-defined plugin names into the audio service
(12:28:06 IST) develneubee: ok now i get it.
(12:28:25 IST) jhe: also you need to do proper boundary checks
(12:28:31 IST) develneubee: yup
(12:28:38 IST) jhe: if you are not in control of the inputs you should never use stuff like strcpy or strcat
(12:28:45 IST) jhe: always use strncpy and strncat
(12:28:56 IST) develneubee: right.
(12:29:07 IST) jhe: same goes for sprintf vs. snprintf
(12:29:40 IST) jhe: also, there's a macro for the max-path length that you should use (instead of the hardcoded 50 that you have now)
(12:29:43 IST) develneubee: so we have to figure out (from the plugin name in audio.conf ) what lib is present in /usr/local/lib/bluetooth ?
(12:30:39 IST) develneubee: ill look at the max-path len macro
(12:30:42 IST) jhe: don't hardcode /usr/local/lib or /usr/lib anywhere
(12:30:48 IST) jhe: e.g. hcid uses something like this:
(12:30:48 IST) jhe: nprintf(command, sizeof(command) - 1, "%s/bluetoothd-service-%s", SERVICEDIR, service->ident);
(12:32:13 IST) develneubee: hmm so its just the path we dont hardcode . we use something like SERVICEDIR
(12:32:34 IST) jhe: where SERVICEDIR depends on the libdir variable in acinclude.m4
(12:32:41 IST) develneubee: yeah
(12:32:59 IST) develneubee: and concatenate it with the string from audio.conf
(12:33:00 IST) jhe: ask marcel how to do that and how acinclude.m4 should be updated
(12:33:21 IST) jhe: yep
(12:33:23 IST) develneubee: why do we need to update acinclude.m4 ?
(12:33:40 IST) develneubee: shouldnt we use SERVICEDIR as it is?
(12:33:41 IST) jhe: if we want a new preprocessor macro for the plugin directory
(12:34:00 IST) jhe: SERVICEDIR is meant for the location of the services
(12:34:11 IST) jhe: you'd need marcel's permission to extend its usage to something else
(12:34:23 IST) develneubee: ohhh right. so something like PLUGINDIR
(12:34:30 IST) jhe: yep
(12:34:41 IST) jhe: by default they'd probably be the same
(12:34:57 IST) develneubee: sure . but they shud be configurable
(12:35:55 IST) jhe: yep
(12:36:42 IST) develneubee: i am skimming through acinclude.m4. i can add the PLUGINDIR thing myself and send a patch to marcel.
(12:36:55 IST) jhe: sure
(12:37:28 IST) develneubee: One more thing, have i put that -ldl CFLAG in the right place ?
(12:37:33 IST) jhe: however check on IRC first that he's fine with the $(libdir)/bluetooth being the default
(12:37:42 IST) jhe: I don't know
(12:37:56 IST) develneubee: looked a bit hackish to me
(12:38:10 IST) jhe: it looks fine to me but marcel always finds some nicer/more correct way of doing these things :)
(12:38:28 IST) develneubee: yeah . ill take his call on this
(12:38:40 IST) jhe: probably he'll want to add some check for the existence of libdl and then automatically ad it to BLUEZ_LDFLAGS