Re: [PATCH 2/2] qmimodem: implement call-forwarding driver

[Denis Kenzior <denkenz@gmail.com>]

Hi Ivaylo,

On 2/15/24 01:17, Ivaylo Dimitrov wrote:
> ---
>   Makefile.am                        |   3 +-
>   drivers/qmimodem/call-forwarding.c | 352 +++++++++++++++++++++++++++++++++++++
>   drivers/qmimodem/voice.h           |   1 +

I fixed up a small whitespace warning flagged by CI:
0002-qmimodem-implement-call-forwarding-driver.patch:394: WARNING: please, no 
space before tabs

>   plugins/gobi.c                     |   3 +
>   4 files changed, 358 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/qmimodem/call-forwarding.c
> 

<snip>

> +
> +struct call_forwarding_info_ext {
> +	uint8_t active;
> +	uint8_t cls;
> +	uint8_t time;
> +	uint8_t pind;
> +	uint8_t sind;
> +	uint8_t type;
> +	uint8_t plan;
> +	uint8_t len;
> +	uint8_t number[0];

Also, changed this to number[] due to:

0002-qmimodem-implement-call-forwarding-driver.patch:80: ERROR: Use C99 flexible 
arrays - see 
https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-arrays

> +};
> +

<snip>

> +static void query_cb(struct qmi_result *result, void *user_data)
> +{
> +	struct cb_data *cbd = user_data;
> +	ofono_call_forwarding_query_cb_t cb = cbd->cb;
> +	const uint8_t *p;
> +	uint8_t num;
> +	uint16_t length;
> +
> +	DBG("");
> +
> +	if (qmi_result_set_error(result, NULL))
> +		goto error;
> +
> +	/*
> +	 * we want extended info, because of the number type.
> +	 * FIXME - shall we fallback to 0x10 if there is no extended info?
> +	 */
> +	p = qmi_result_get(result, 0x16, &length);
> +	if (p) {
> +		struct ofono_call_forwarding_condition *list;
> +		const uint8_t *end = p + length;
> +		int i;
> +
> +		num = *p++;

Can length be 0?  If so, feel free to send a fix :)

> +
> +		list = g_new0(struct ofono_call_forwarding_condition, num);
> +
> +		for (i = 0; i < num; i++) {
> +			struct call_forwarding_info_ext *info = (void *)p;
> +			int type;
> +
> +			/* do not try to access beyond buffer end */
> +			if (p + sizeof(*info) > end ||
> +					p + sizeof(*info) + info->len > end) {
> +				g_free(list);
> +				goto error;
> +			}
> +
> +			if (info->type == 1)
> +				type = OFONO_NUMBER_TYPE_INTERNATIONAL;
> +			else
> +				type = OFONO_NUMBER_TYPE_UNKNOWN;
> +
> +			set_fwd_cond(&list[i], info->active, info->cls,
> +					info->time, type, info->number,
> +					info->len);
> +			p += sizeof(*info) + info->len;
> +		}
> +
> +		CALLBACK_WITH_SUCCESS(cb, num, list, cbd->data);
> +		g_free(list);
> +		return;
> +	}
> +
> +error:
> +	CALLBACK_WITH_FAILURE(cb, 0, NULL, cbd->data);
> +}
> +

Applied, thanks.

Regards,
-Denis