Patch to set authentication data

Aleksander Morgado aleksander at lanedo.com
Wed Jan 9 01:22:39 PST 2013


Hey!

Review below.

> 
> diff --git a/cli/qmicli-wds.c b/cli/qmicli-wds.c
> index 775824b..2079338 100644
> --- a/cli/qmicli-wds.c
> +++ b/cli/qmicli-wds.c
> @@ -57,8 +57,8 @@ static gboolean noop_flag;
>  
>  static GOptionEntry entries[] = {
>      { "wds-start-network", 0, 0, G_OPTION_ARG_STRING, &start_network_str,
> -      "Start network",
> -      "[APN]"
> +      "Start network (Username and Password are optional)",
> +      "[APN|Username|Password]"


Other commands with multiple parameters passed in the same string would
use plain commas to separate the items, so better use commas, e.g.:
  --dms-set-user-lock-code=[(old lock code),(new lock code)]

The pipe/OR sign "|" (however is called) is used in the documentation of
the commands to show a closed set of options to use; e.g.:
  --dms-uim-verify-pin=[(PIN|PIN2),(current PIN)]

Which means that you can pass either "PIN" or "PIN2"; e.g. "PIN|1234" or
"PIN2,1234" for example.


>      },
>      { "wds-follow-network", 0, 0, G_OPTION_ARG_NONE, &follow_network_flag,
>        "Follow the network status until disconnected. Use with `--wds-start-network'",
> @@ -593,17 +593,32 @@ qmicli_wds_run (QmiDevice *device,
>      /* Request to start network? */
>      if (start_network_str) {
>          QmiMessageWdsStartNetworkInput *input = NULL;
> +    	gchar **split;

Please don't use tabs; indent and align with spaces always.

> +        QmiWdsAuthentication qmiwdsauth;
>  
>          /* Use the input string as APN */
>          if (start_network_str[0]) {
> +	    split = g_strsplit (start_network_str, "|", 0);
>              input = qmi_message_wds_start_network_input_new ();
> -            qmi_message_wds_start_network_input_set_apn (input, start_network_str, NULL);
> +            qmi_message_wds_start_network_input_set_apn (input, split[0], NULL);
> +	    if (split[1] && split[2]) {
> +	            qmi_message_wds_start_network_input_set_username (input, split[1], NULL);
> +		    qmiwdsauth = QMI_WDS_AUTHENTICATION_PAP;
> +	            qmi_message_wds_start_network_input_set_authentication_preference (input, qmiwdsauth, NULL);
> +	            qmi_message_wds_start_network_input_set_password (input, split[2], NULL);
> +	    } else if (split[1]) {

It would probably be cleaner to have:
  if (split[1]) {
       /* add split[1] */
       if (split[2]) {
           /* add split[2] */
       }
  }

> +	            qmi_message_wds_start_network_input_set_username (input, split[1], NULL);
> +		    qmiwdsauth = QMI_WDS_AUTHENTICATION_PAP;
> +	            qmi_message_wds_start_network_input_set_authentication_preference (input, qmiwdsauth, NULL);
> +	            qmi_message_wds_start_network_input_set_password (input, "", NULL);
> +	    }
> +	    g_strfreev (split);


I'm missing the possibility to use CHAP authentication. Could you update
the command so that we can also pass "chap" or "pap"? Something like:
--wds-start-network=[(APN),(chap|pap),(user),(pass)]



>          }
>  
>          g_debug ("Asynchronously starting network...");
>          qmi_client_wds_start_network (ctx->client,
>                                        input,
> -                                      10,
> +                                      45,

Could you give the timeout update in a separate patch?

>                                        ctx->cancellable,
>                                        (GAsyncReadyCallback)start_network_ready,
>                                        NULL);


Cheers!

-- 
Aleksander


More information about the libqmi-devel mailing list