Re: [PATCH 2/8] stkagent: '+' is considered as a digit

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2255 bytes --]

Hi Philippe,

On 08/22/2012 11:18 AM, Philippe Nunes wrote:
> The function 'valid_phone_number_format' does not comply with GCF test cases
> expectation. For STK, the character '+' should be considered as a whole digit.
> ---
>   src/stkagent.c |   26 ++++++++++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/src/stkagent.c b/src/stkagent.c
> index 7c3f697..392a2b2 100644
> --- a/src/stkagent.c
> +++ b/src/stkagent.c
> @@ -69,6 +69,27 @@ struct stk_agent {
>   #define TERMINATE_ERROR ERROR_PREFIX ".EndSession"
>   #define BUSY_ERROR ERROR_PREFIX ".Busy"
>
> +static gboolean check_digit(const char *digit)

I really don't like the name...

> +{
> +	int len = strlen(digit);
> +	int i;
> +
> +	if (!len)
> +		return FALSE;
> +
> +	for (i = 0; i<  len; i++) {
> +		if (digit[i]>= '0'&&  digit[i]<= '9')
> +			continue;
> +
> +		if (digit[i] == '*' || digit[i] == '#' || digit[i] == '+')
> +			continue;
> +
> +		return FALSE;
> +	}
> +
> +	return TRUE;

And this function is entirely too long-winded for what it is doing. 
Please refer to 'man strspn'.  It might be easier to simply use strspn 
below...

> +}
> +
>   static void stk_agent_send_noreply(struct stk_agent *agent, const char *method)
>   {
>   	DBusConnection *conn = ofono_dbus_get_connection();
> @@ -541,7 +562,7 @@ static void get_digit_cb(DBusPendingCall *call, void *data)
>   					DBUS_TYPE_STRING,&digit,
>   					DBUS_TYPE_INVALID) == FALSE ||
>   			strlen(digit) != 1 ||
> -			!valid_phone_number_format(digit)) {
> +			!check_digit(digit)) {

This is still wrong as it also needs to take care of the hidden_input 
case where the '+' is not allowed.

>   		ofono_error("Can't parse the reply to GetDigit()");
>   		remove_agent = TRUE;
>   		goto error;
> @@ -675,7 +696,8 @@ static void get_digits_cb(DBusPendingCall *call, void *data)
>
>   	if (dbus_message_get_args(reply, NULL,
>   					DBUS_TYPE_STRING,&string,
> -					DBUS_TYPE_INVALID) == FALSE) {
> +					DBUS_TYPE_INVALID) == FALSE ||
> +					!check_digit(string)) {

As above

>   		ofono_error("Can't parse the reply to GetDigits()");
>   		remove_agent = TRUE;
>   		goto error;

Regards,
-Denis