gsm 7bit decode: *8/7

[Neels Hofmeyr]

Hi list,

I just thought I had fixed gsm_7bit_decode_n(), since its API doc says

   * \param length        The length of the input sequence (in octets).
  int gsm_7bit_decode_n([...], uint8_t length);

but the implementation is

  int gsm_7bit_decode_n([...], uint8_t septet_l)

and indeed the length parameter is handled as septet length. So, logically, I
came up with this patch:


diff --git a/src/gsm/gsm_utils.c b/src/gsm/gsm_utils.c
index e8e452f..a9afc1a 100644
--- a/src/gsm/gsm_utils.c
+++ b/src/gsm/gsm_utils.c
@@ -184,8 +184,9 @@
  return text - text_buf_begin;
 }
 
-int gsm_7bit_decode_n(char *text, size_t n, const uint8_t *user_data, uint8_t septet_l)
+int gsm_7bit_decode_n(char *text, size_t n, const uint8_t *user_data, uint8_t octet_l)
 {
+ uint8_t septet_l = ((uint16_t)octet_l * 8) / 7;
  return gsm_7bit_decode_n_hdr(text, n, user_data, septet_l, 0);
 }
 

(the cast to uint16_t makes 100% sure the calculation isn't truncated to
uint8_t after the *8 multiplication -- I picked this up while hacking on 8bit
microcontrollers.  That truncation doesn't really happen on our 32bit/64bit
machines, but if it did, the length would effectively be limited to 255/8 == 31
characters.)

Then I noticed that callers actually do *8/7 before calling the function:

  parse_process_uss_req():
    num_chars = (uss_req_data[6] * 8) / 7;
    /* Prevent a mobile-originated buffer-overrun! */
    if (num_chars > MAX_LEN_USSD_STRING)
            num_chars = MAX_LEN_USSD_STRING;
    gsm_7bit_decode_n_ussd((char *)req->ussd_text,
                            sizeof(req->ussd_text),
                            &(uss_req_data[7]), num_chars);

(This is gsm_7bit_decode_n_ussd(), its API doc says "see gsm_7bit_decode_n()")

So we have the API talking about length "in octets" while the implementation
clearly employs septet length.

Fixing the implementation to match the API doc would break binary
compatibility. I should probably fix the API doc to match that weird
implementation, but it nags and hurts a little.

Any opinions?

~Neels


-- 
- Neels Hofmeyr <nhofmeyr at sysmocom.de>          http://www.sysmocom.de/
=======================================================================
* sysmocom - systems for mobile communications GmbH
* Alt-Moabit 93
* 10559 Berlin, Germany
* Sitz / Registered office: Berlin, HRB 134158 B
* Geschäftsführer / Managing Directors: Holger Freyther, Harald Welte
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.osmocom.org/pipermail/openbsc/attachments/20160229/470cab38/attachment.bin>