[PATCH 1/2] Add Android socket support for control part

[Dmitry Shmidt]

On Fri, Feb 25, 2011 at 12:37 AM, Johannes Berg
<johannes at sipsolutions.net> wrote:
> On Thu, 2011-02-24 at 16:00 -0800, Dmitry Shmidt wrote:
>
>> ?#if defined(CONFIG_CTRL_IFACE_UNIX) || defined(CONFIG_CTRL_IFACE_UDP)
>> ?#define CTRL_IFACE_SOCKET
>> +#ifdef ANDROID
>> +static const char *local_socket_dir = "/data/misc/wifi/sockets";
>> +static const char *local_socket_prefix = "wpa_ctrl_";
>> +#endif /* ANDROID */
>
> I think this shouldn't be hardcoded. IMO it would make a lot more sense
> to allow configuring this more flexibly, and then make Android use the
> default config. Otherwise, we will end up with tons of different ifdefs
> here.

Despite it is only one place, you are right, we can set it in .config
The problem is that Android is using special sockets (for security
reasons) that are
not "standard".

>
>> +#ifdef ANDROID
>> +/**
>> + * wpa_ctrl_cleanup() - Delete any local UNIX domain socket files that
>> + * may be left over from clients that were previously connected to
>> + * wpa_supplicant. This keeps these files from being orphaned in the
>> + * event of crashes that prevented them from being removed as part
>> + * of the normal orderly shutdown.
>> + */
>> +void wpa_ctrl_cleanup(void)
>> +{
>> + ? ?DIR *dir;
>> + ? ?struct dirent entry;
>> + ? ?struct dirent *result;
>> + ? ?size_t dirnamelen;
>> + ? ?int prefixlen = strlen(local_socket_prefix);
>> + ? ?size_t maxcopy;
>> + ? ?char pathname[PATH_MAX];
>> + ? ?char *namep;
>> +
>> + ? ?if ((dir = opendir(local_socket_dir)) == NULL)
>> + ? ? ? ?return;
>> +
>> + ? ?dirnamelen = (size_t)snprintf(pathname, sizeof(pathname), "%s/",
>> local_socket_dir);
>> + ? ?if (dirnamelen >= sizeof(pathname)) {
>> + ? ? ? ?closedir(dir);
>> + ? ? ? ?return;
>> + ? ?}
>> + ? ?namep = pathname + dirnamelen;
>> + ? ?maxcopy = PATH_MAX-dirnamelen;
>> + ? ?while (readdir_r(dir, &entry, &result) == 0 && result != NULL) {
>> + ? ? ? ?if (strncmp(entry.d_name, local_socket_prefix, prefixlen) == 0) {
>> + ? ? ? ? ? ?if (strlcpy(namep, entry.d_name, maxcopy) < maxcopy) {
>> + ? ? ? ? ? ? ? ?unlink(pathname);
>> + ? ? ? ? ? ?}
>> + ? ? ? ?}
>> + ? ?}
>> + ? ?closedir(dir);
>> +}
>> +#endif /* ANDROID */
>>
>> +#else /* CONFIG_CTRL_IFACE_UNIX */
>> +#ifdef ANDROID
>> +void wpa_ctrl_cleanup(void)
>> +{
>
> The ifdef'ing here is quite a mess -- but why is this necessary anyway?
> Leftover sockets can't be connected to, and when a new wpa_s starts up
> it should already attempt this and clean up?

We found in some cases we need to clean the sockets. I can not prove
it easily, but
truly there is no reason it is under ANDROID ifdef - other systems
will "enjoy" this as well.
Unless you have strong argument why not.

>
>> @@ -234,7 +323,11 @@ int wpa_ctrl_request(struct wpa_ctrl *ctrl, const
>> char *cmd, size_t cmd_len,
>> ? ? ? os_free(cmd_buf);
>>
>> ? ? ? for (;;) {
>> +#ifdef ANDROID
>> + ? ? ? ? ? ? tv.tv_sec = 10;
>> +#else
>> ? ? ? ? ? ? ? tv.tv_sec = 2;
>> +#endif
>
> This seems like it could be there all the time? Needs a separate patch
> anyway.

You mean we can just change it to 10 sec for everybody ?

>
>> @@ -56,6 +56,12 @@ extern "C" {
>> ?#define WPA_EVENT_EAP_FAILURE "CTRL-EVENT-EAP-FAILURE "
>> ?/** New scan results available */
>> ?#define WPA_EVENT_SCAN_RESULTS "CTRL-EVENT-SCAN-RESULTS "
>> +/** wpa_supplicant state change */
>> +#define WPA_EVENT_STATE_CHANGE "CTRL-EVENT-STATE-CHANGE "
>> +/** AP to STA speed */
>> +#define WPA_EVENT_LINK_SPEED "CTRL-EVENT-LINK-SPEED "
>> +/** Driver state change */
>> +#define WPA_EVENT_DRIVER_STATE "CTRL-EVENT-DRIVER-STATE "
>> ?/** A new BSS entry was added (followed by BSS entry id and BSSID) */
>> ?#define WPA_EVENT_BSS_ADDED "CTRL-EVENT-BSS-ADDED "
>> ?/** A BSS entry was removed (followed by BSS entry id and BSSID) */
>
> definitely doesn't belong into this patch
>
You are right, my mistake.

> johannes
>
Thanks,

Dmitry