Open Source Telephony
 help / color / mirror / Atom feed
From: Denis Kenzior <denkenz@gmail.com>
To: ofono@ofono.org
Subject: Re: [PATCH 1/7] Include datagram support to history API
Date: Mon, 27 Sep 2010 21:16:51 -0500	[thread overview]
Message-ID: <4CA15013.9070009@gmail.com> (raw)
In-Reply-To: <1285606459-1297-2-git-send-email-aki.niemi@nokia.com>

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

Hi Aki,

> --- a/src/history.c
> +++ b/src/history.c
> @@ -47,7 +47,15 @@ struct history_call_foreach_data {
>  struct history_sms_foreach_data {
>  	const struct ofono_uuid *uuid;
>  	const char *address;
> -	const char *text;
> +	union {
> +		const char *text;
> +		struct {
> +			const unsigned char *data;
> +			unsigned len;
> +			guint16 dst;
> +			guint16 src;

Are you overflowing the dst/src ports for the 16 bit case by any chance?
 With the 8bit port fix patch, we're actually using 24 bits to store
these values.  There might need to be a comment in the history API about
this.

Are 8 bit ports even used in practice?  If not, then might be that
shifting 8-bit values by 16 is better...

Other than that, I'm happy with this one.

Regards,
-Denis

  reply	other threads:[~2010-09-28  2:16 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-27 16:54 SMS agent interface Aki Niemi
2010-09-27 16:54 ` [PATCH 1/7] Include datagram support to history API Aki Niemi
2010-09-28  2:16   ` Denis Kenzior [this message]
2010-09-27 16:54 ` [PATCH 2/7] Add datagram dispatch to sms atom Aki Niemi
2010-09-27 16:54 ` [PATCH 3/7] Add message agent interface documentation Aki Niemi
2010-09-29  2:27   ` Denis Kenzior
2010-09-29  5:56     ` Aki Niemi
2010-09-29  9:52       ` Marcel Holtmann
2010-09-29 12:45         ` Aki Niemi
2010-09-29 14:10           ` Marcel Holtmann
2010-09-27 16:54 ` [PATCH 4/7] Add message agent interface definition Aki Niemi
2010-09-27 16:54 ` [PATCH 5/7] Add message agent implementation Aki Niemi
2010-09-27 16:54 ` [PATCH 6/7] Add agent interface to sms atom Aki Niemi
2010-09-27 16:54 ` [PATCH 7/7] Add test sms agent implementation Aki Niemi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4CA15013.9070009@gmail.com \
    --to=denkenz@gmail.com \
    --cc=ofono@ofono.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox