Re: [PATCH] implemented GSM340_TP_VPF_ABSOLUTE

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