Re: [Openvpn-devel] [[Patch v3] 3/3] Android platform specific changes.

[Gert D. <ge...@gr...>]

Hi,

On Fri, Apr 26, 2013 at 09:54:05PM +0200, Arne Schwabe wrote:
> +/*
> + * The android control method will instruct the GUI part of openvpn to
> + * do the route/ifconfig/open tun command. 
> + */
> +bool management_android_control (struct management *man, const char *command, const char *msg)
> +{
> +  struct user_pass up;    
> +  strcpy(up.username, msg);
> +  
> +  management_query_user_pass(management, &up , command, GET_USER_PASS_NEED_OK,(void*) 0);
> +  return strcmp ("ok", up.password)==0;
> +}
>  #endif

Much better :-) - but if I may voice some extra wishes... :-)

  CLEAR(up);
  strncp(up.username, msg, sizeof(up.username)-1);

"just to be sure"

> diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
> index 8eb112b..ade9aad 100644
> --- a/src/openvpn/socket.c
> +++ b/src/openvpn/socket.c
> @@ -725,6 +726,14 @@ create_socket (struct link_socket *sock)
>      {
>        ASSERT (0);
>      }
> +#ifdef TARGET_ANDROID
> +    struct user_pass up;
> +
> +    management->connection.fdtosend = sock->sd;
> +    management_android_control (management, "PROTECTFD", __func__);
> +
> +#endif
> +    

A stray "struct user_pass" is dangling around here :-) - and I think this
code would be helped by a comment

   /* pass socket FD to management interface to pass on to VPN API 
    * as "protected socket" (exempt from being routed into tunnel)
    */

[..]
> +  /* Set the actual name to a dummy name */
> +  tt->actual_name = malloc(sizeof(ANDROID_TUNNAME));
> +  strcpy(tt->actual_name, ANDROID_TUNNAME);

Uh... strdup()?  And malloc return value checking?

... but besides these, I think we're getting close :-)

gert
-- 
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             ge...@gr...
fax: +49-89-35655025                        ge...@ne...