[PATCH] implemented GSM340_TP_VPF_ABSOLUTE

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

Harald Welte laforge at gnumonks.org
Sun Nov 8 05:39:02 UTC 2009


Dear Steffen,

thanks for your patch, it is most welcome.  Sorry for not providing feedback
earlier.   Please see my comments below:

> -	case GSM340_TP_VPF_ABSOLUTE:
> +	case GSM340_TP_VPF_ABSOLUTE: ;
>  		/* Chapter 9.2.3.12.2 */
> -		/* FIXME: like service center time stamp */
> -		DEBUGP(DSMS, "VPI absolute not implemented yet\n");
> +		time_t expires = gsm340_scts(sms_vp);
> +		time_t now = mktime(gmtime(NULL));
> +		if (expires <= now)
> +			minutes = 0;
> +		else
> +			minutes = (expires-now)/60;

please declare the variables (expires/now) at the beginning of the section,
e.g. the case statement or even the top of the function.

> +/* Turn semi-octet representation into int: 0x89 => 98 */
> +static u_int8_t unbcdify(u_int8_t value)
> +{
> +	u_int8_t ret;
> +
> +	ret = (value&0x0F)*10;
> +	ret += value>>4;
> +
> +	return ret;
> +}

we might want to have some kind of treatment (or at least an error message)
if the input BCD nibbles exceed the 0...9 range.

>  static void gsm340_gen_scts(u_int8_t *scts, time_t time)
>  {
> -	struct tm *tm = localtime(&time);
> +	struct tm *tm = gmtime(&time);
>  
>  	*scts++ = bcdify(tm->tm_year % 100);
>  	*scts++ = bcdify(tm->tm_mon + 1);
> @@ -338,7 +356,30 @@ static void gsm340_gen_scts(u_int8_t *scts, time_t time)
>  	*scts++ = bcdify(tm->tm_hour);
>  	*scts++ = bcdify(tm->tm_min);
>  	*scts++ = bcdify(tm->tm_sec);
> -	*scts++ = 0;	/* FIXME: timezone */
> +	*scts++ = 0;	/* FIXME: local timezone */
> +}

mh. We are no wending GMT but no timezone information, which I'm not quite sure
if it is better than we did before.  A proper solution would be to use
the signed value of tm->tm_gmtoff/(60*15);

> +/* Decode 03.40 TP-SCTS (into utc/gmt timestamp) */
> +static time_t gsm340_scts(u_int8_t *scts)
> +{
> +	struct tm tm;
> +	
> +	u_int8_t yr = unbcdify(*scts++);
> +	printf("%i",yr);
> +	if (yr <= 60)
> +		tm.tm_year = 100 + yr;
> +	else
> +		tm.tm_year = yr;

is this year '60' specified anywhere in the GSM spec, or did you just introduce
it yourself?  In the latter case, I would rather use something like 80, since
GSM was not specified before the eighties.

Thanks again,
	Harald
-- 
- Harald Welte <laforge at gnumonks.org>           http://laforge.gnumonks.org/
============================================================================
"Privacy in residential applications is a desirable marketing option."
                                                  (ETSI EN 300 175-7 Ch. A6)




More information about the OpenBSC mailing list