[PATCH 0/3] Rebase dc-test to master.

[Octavio Alvarez]

This is an attempt to rebase Felipe Contreras' dc-test branch
to current code.

The goal is to allow "make DIRECTCONN=y" to successfully
complete, and include all of his fixes from back then.

It is not intended make fast file transfer work.

Felipe Contreras (1):
directconn: fix channel encoding

Octavio Alvarez (2):
directconn: build fixes.
directconn: non-build fixes that were left around.

cvr/pn_direct_conn.c | 19 +++++++++++++------
cvr/pn_direct_conn.h | 2 +-
cvr/pn_peer_link.c | 14 ++++++++++++++
cvr/pn_peer_link.h | 5 +++++
cvr/pn_peer_msg.c | 7 +++----
msn.c | 4 ++++
6 files changed, 40 insertions(+), 11 deletions(-)

[Octavio Alvarez]

Rebased dc-test branch from Felipe Contreras
in order to make those compile under current
code.

call->type check was removed after commit bcef8cb2
---
cvr/pn_direct_conn.c | 13 +++++++------

cvr/pn_direct_conn.h | 2 +-
cvr/pn_peer_link.c | 14 ++++++++++++++
cvr/pn_peer_link.h | 5 +++++
cvr/pn_peer_msg.c | 7 +++----
msn.c | 4 ++++

6 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/cvr/pn_direct_conn.c b/cvr/pn_direct_conn.c
index 21459c6..11e4b48 100644
--- a/cvr/pn_direct_conn.c
+++ b/cvr/pn_direct_conn.c
@@ -23,6 +23,7 @@

#include "session.h"
#include "pn_peer_msg.h"
+#include "pn_peer_link.h"

#include "io/pn_node.h"

@@ -50,7 +51,7 @@ pn_direct_conn_send_handshake(struct pn_direct_conn *direct_conn)

link = direct_conn->link;

- peer_msg = pn_peer_msg_new(link);
+ peer_msg = pn_peer_msg_new();
peer_msg->flags = 0x100;

if (direct_conn->nonce != NULL)
@@ -408,7 +409,7 @@ pn_direct_conn_connect(struct pn_direct_conn *direct_conn, const char *host, int

pn_log ("begin");

- session = direct_conn->link->session;
+ session = pn_peer_link_get_session(direct_conn->link);

#if 0
if (session->http_method)
@@ -417,7 +418,7 @@ pn_direct_conn_connect(struct pn_direct_conn *direct_conn, const char *host, int
}
#endif

- direct_conn->connect_data = purple_proxy_connect(NULL, msn_session_get_account (session),
+ direct_conn->connect_data = purple_proxy_connect(NULL, msn_session_get_user_data (session),
host, port, direct_conn_connect_cb, direct_conn);

pn_log ("end");
@@ -458,10 +459,10 @@ pn_direct_conn_new(struct pn_peer_link *link)

direct_conn->link = link;

- if (link->direct_conn != NULL)
+ if (pn_peer_link_get_directconn(link) != NULL)
pn_warning ("got_transresp: LEAK");

- link->direct_conn = direct_conn;
+ pn_peer_link_set_directconn(link, direct_conn);

pn_log ("end");

@@ -492,7 +493,7 @@ pn_direct_conn_destroy(struct pn_direct_conn *direct_conn)
if (direct_conn->nonce != NULL)
g_free(direct_conn->nonce);

- direct_conn->link->direct_conn = NULL;
+ pn_peer_link_set_directconn(direct_conn->link, NULL);

g_free(direct_conn);

diff --git a/cvr/pn_direct_conn.h b/cvr/pn_direct_conn.h
index 3d203da..38a5d63 100644
--- a/cvr/pn_direct_conn.h
+++ b/cvr/pn_direct_conn.h
@@ -44,7 +44,7 @@ struct pn_direct_conn {

int c;
struct _PurpleProxyConnectData *connect_data;
- PecanStream *stream;
+ PnStream *stream;
};

struct pn_direct_conn *pn_direct_conn_new(struct pn_peer_link *link);
diff --git a/cvr/pn_peer_link.c b/cvr/pn_peer_link.c
index 162c7db..b842ca8 100644
--- a/cvr/pn_peer_link.c
+++ b/cvr/pn_peer_link.c
@@ -789,3 +789,17 @@ pn_peer_link_request_object(struct pn_peer_link *link,

g_free(msnobj_base64);
}
+
+struct pn_direct_conn *
+pn_peer_link_get_directconn(const struct pn_peer_link *link)
+{
+ return link->direct_conn;
+}
+
+void
+pn_peer_link_set_directconn(struct pn_peer_link *link, struct pn_direct_conn *direct_conn)
+{
+ link->direct_conn = direct_conn;
+}
+
+
diff --git a/cvr/pn_peer_link.h b/cvr/pn_peer_link.h
index 66ccd67..28d2621 100644
--- a/cvr/pn_peer_link.h
+++ b/cvr/pn_peer_link.h
@@ -31,6 +31,8 @@ struct MsnMessage;

struct _PurpleXfer;

+struct pn_peer_link;
+
typedef void (*MsnSlpCb) (struct pn_peer_call *call, const guchar *data, gsize size);
typedef void (*MsnSlpEndCb) (struct pn_peer_call *call, struct MsnSession *session);

@@ -70,4 +72,7 @@ struct pn_peer_link *msn_session_find_peer_link(struct MsnSession *session,
struct pn_peer_link *msn_session_get_peer_link(struct MsnSession *session,
const char *username);

+struct pn_direct_conn *pn_peer_link_get_directconn(const struct pn_peer_link *link);
+void pn_peer_link_set_directconn(struct pn_peer_link *link, struct pn_direct_conn *direct_Conn);
+
#endif /* PN_PEERLINK_H */
diff --git a/cvr/pn_peer_msg.c b/cvr/pn_peer_msg.c
index 31bfb9a..5e5ad0d 100644
--- a/cvr/pn_peer_msg.c
+++ b/cvr/pn_peer_msg.c
@@ -698,11 +698,10 @@ got_ok(struct pn_peer_call *call,

if (strcmp(type, "application/x-msnmsgr-sessionreqbody") == 0) {
#ifdef MSN_DIRECTCONN
- if (call->link->session->use_direct_conn &&
- call->type == PN_PEER_CALL_DC)
+
+ if (msn_session_get_bool (pn_peer_link_get_session(call->link), "use_direct_conn"))
{
/* First let's try a DirectConnection. */
-
struct pn_peer_link *link;
struct pn_peer_msg *peer_msg;
char *header;
@@ -720,7 +719,7 @@ got_ok(struct pn_peer_call *call,
"ICF: false\r\n");

header = g_strdup_printf("INVITE MSNMSGR:%s MSNSLP/1.0",
- link->remote_user);
+ pn_peer_link_get_passport(link));

peer_msg = sip_new(call, 0, header, branch,
"application/x-msnmsgr-transreqbody",
diff --git a/msn.c b/msn.c
index 25c982d..67ebb44 100644
--- a/msn.c
+++ b/msn.c
@@ -906,8 +906,10 @@ login (PurpleAccount *account)
session->user_data = account;
msn_session_set_bool (session, "use_server_alias",
purple_account_get_bool (account, "use_server_alias", FALSE));
+#ifdef MSN_DIRECTCONN
msn_session_set_bool (session, "use_direct_conn",
purple_account_get_bool (account, "use_direct_conn", FALSE));
+#endif
msn_session_set_bool (session, "use_userdisplay",
purple_account_get_bool (account, "use_userdisplay", TRUE));

@@ -1844,8 +1846,10 @@ init_plugin (PurplePlugin *plugin)
option = purple_account_option_bool_new (_("Use server-side alias"), "use_server_alias", FALSE);
prpl_info.protocol_options = g_list_append (prpl_info.protocol_options, option);

+#ifdef MSN_DIRECTCONN
option = purple_account_option_bool_new (_("Use direct connections"), "use_direct_conn", FALSE);
prpl_info.protocol_options = g_list_append (prpl_info.protocol_options, option);
+#endif

option = purple_account_option_bool_new (_("Use user displays"), "use_userdisplay", TRUE);
prpl_info.protocol_options = g_list_append (prpl_info.protocol_options, option);
--
1.6.5.3

[Octavio Alvarez]

From: Felipe Contreras <felipe.c...@gmail.com>

Signed-off-by: Felipe Contreras <felipe.c...@gmail.com>
---
cvr/pn_direct_conn.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/cvr/pn_direct_conn.c b/cvr/pn_direct_conn.c
index ae8f53f..986bf5b 100644
--- a/cvr/pn_direct_conn.c
+++ b/cvr/pn_direct_conn.c
@@ -378,6 +378,8 @@ connect_cb(gpointer data, gint source, const gchar *error_message)
direct_conn->stream = pn_stream_new (fd);
channel = direct_conn->stream->channel;

+ g_io_channel_set_encoding (channel, NULL, NULL);
+
pn_info ("connected: %p", channel);
direct_conn->read_watch = g_io_add_watch (channel, G_IO_IN, read_cb, direct_conn);

--
1.6.5.3

[Felipe Contreras]

On Fri, Dec 4, 2009 at 10:01 AM, Octavio Alvarez
<alva...@alvarezp.ods.org> wrote:
> Rebased dc-test branch from Felipe Contreras
> in order to make those compile under current
> code.

While that's true, many changes were done by you, so instead of saying
just "Rebased from Foo", say "Based on patches from Foo"

> call->type check was removed after commit bcef8cb2

I didn't get that.

I'm not sure of the code-style anymore, but I think each function
argument is on a separate line, specially on functions with such long
names.

> +{
> +    link->direct_conn = direct_conn;
> +}
> +
> +

Two unnecessary extra lines.

> diff --git a/cvr/pn_peer_link.h b/cvr/pn_peer_link.h
> index 66ccd67..28d2621 100644
> --- a/cvr/pn_peer_link.h
> +++ b/cvr/pn_peer_link.h
> @@ -31,6 +31,8 @@ struct MsnMessage;
>
>  struct _PurpleXfer;
>
> +struct pn_peer_link;
> +

I don't think this is actually needed, and it's unrelated to this
patch, but it's a good change anyway. I think the same should be done
for other structures in the cvr subdir, can you write a separate patch
for that?

>  typedef void (*MsnSlpCb) (struct pn_peer_call *call, const guchar *data, gsize size);
>  typedef void (*MsnSlpEndCb) (struct pn_peer_call *call, struct MsnSession *session);
>
> @@ -70,4 +72,7 @@ struct pn_peer_link *msn_session_find_peer_link(struct MsnSession *session,
>  struct pn_peer_link *msn_session_get_peer_link(struct MsnSession *session,
>                                                const char *username);
>
> +struct pn_direct_conn *pn_peer_link_get_directconn(const struct pn_peer_link *link);
> +void pn_peer_link_set_directconn(struct pn_peer_link *link, struct pn_direct_conn *direct_Conn);
> +
>  #endif /* PN_PEERLINK_H */
> diff --git a/cvr/pn_peer_msg.c b/cvr/pn_peer_msg.c
> index 31bfb9a..5e5ad0d 100644
> --- a/cvr/pn_peer_msg.c
> +++ b/cvr/pn_peer_msg.c
> @@ -698,11 +698,10 @@ got_ok(struct pn_peer_call *call,
>
>     if (strcmp(type, "application/x-msnmsgr-sessionreqbody") == 0) {
>  #ifdef MSN_DIRECTCONN
> -        if (call->link->session->use_direct_conn &&
> -            call->type == PN_PEER_CALL_DC)
> +
> +        if (msn_session_get_bool (pn_peer_link_get_session(call->link), "use_direct_conn"))

This seems to be weirdly formatted.

>         {
>             /* First let's try a DirectConnection. */
> -

No need for that line.

The rest of the patch looks nice :)

Cheers.

--
Felipe Contreras

[Octavio Alvarez]

On Fri, 04 Dec 2009 02:27:45 -0800, Felipe Contreras
<felipe.c...@gmail.com> wrote:

>> +struct pn_peer_link;
>> +
>
> I don't think this is actually needed, and it's unrelated to this
> patch, but it's a good change anyway. I think the same should be done
> for other structures in the cvr subdir, can you write a separate patch
> for that?

Sure.

In this case it is needed to build. Otherwise I would have to include
the definition in the .h by any mean, which would make the struct
public.

>> +void
>> +pn_peer_link_set_directconn(struct pn_peer_link *link, struct
>> pn_direct_conn *direct_conn)
>
> I'm not sure of the code-style anymore, but I think each function
> argument is on a separate line, specially on functions with such long
> names.
>
>> +

>> + if (msn_session_get_bool
>> (pn_peer_link_get_session(call->link), "use_direct_conn"))
>
> This seems to be weirdly formatted.

Are you using any particular coding style?

[Octavio Alvarez]

This is version 2 of the attempt to rebase Felipe

Contreras' dc-test branch to current code.

The goal is to allow "make DIRECTCONN=y" to successfully
complete, and include all of his fixes from back then.

It is not intended make fast file transfer work.

As version 2 of the patch set, it provides better
code formatting and squashes patch #3 into #2.
This makes the first patch a build fixer and the
second one a bug fixer.

Octavio Alvarez (2):
directconn: build fixes.

directconn: generic bug fixes

cvr/pn_direct_conn.c | 20 ++++++++++++++------
cvr/pn_direct_conn.h | 2 +-
cvr/pn_peer_link.c | 13 +++++++++++++
cvr/pn_peer_link.h | 6 ++++++
cvr/pn_peer_msg.c | 7 ++++---
msn.c | 4 ++++
6 files changed, 42 insertions(+), 10 deletions(-)

[Octavio Alvarez]

Based on previous patches from Felipe Contreras.
---
cvr/pn_direct_conn.c | 14 ++++++++------

cvr/pn_direct_conn.h | 2 +-
cvr/pn_peer_link.c | 13 +++++++++++++
cvr/pn_peer_link.h | 6 ++++++
cvr/pn_peer_msg.c | 7 ++++---
msn.c | 4 ++++

6 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/cvr/pn_direct_conn.c b/cvr/pn_direct_conn.c
index 21459c6..99d3f5c 100644

--- a/cvr/pn_direct_conn.c
+++ b/cvr/pn_direct_conn.c
@@ -23,6 +23,7 @@

#include "session.h"
#include "pn_peer_msg.h"
+#include "pn_peer_link.h"

#include "io/pn_node.h"

@@ -50,7 +51,7 @@ pn_direct_conn_send_handshake(struct pn_direct_conn *direct_conn)

link = direct_conn->link;

- peer_msg = pn_peer_msg_new(link);
+ peer_msg = pn_peer_msg_new();
peer_msg->flags = 0x100;

if (direct_conn->nonce != NULL)
@@ -408,7 +409,7 @@ pn_direct_conn_connect(struct pn_direct_conn *direct_conn, const char *host, int

pn_log ("begin");

- session = direct_conn->link->session;
+ session = pn_peer_link_get_session(direct_conn->link);

#if 0
if (session->http_method)

@@ -417,7 +418,8 @@ pn_direct_conn_connect(struct pn_direct_conn *direct_conn, const char *host, int

}
#endif

- direct_conn->connect_data = purple_proxy_connect(NULL, msn_session_get_account (session),
+ direct_conn->connect_data = purple_proxy_connect(NULL,

+ msn_session_get_user_data(session),

host, port, direct_conn_connect_cb, direct_conn);

pn_log ("end");

@@ -458,10 +460,10 @@ pn_direct_conn_new(struct pn_peer_link *link)

direct_conn->link = link;

- if (link->direct_conn != NULL)
+ if (pn_peer_link_get_directconn(link) != NULL)
pn_warning ("got_transresp: LEAK");

- link->direct_conn = direct_conn;
+ pn_peer_link_set_directconn(link, direct_conn);

pn_log ("end");

@@ -492,7 +494,7 @@ pn_direct_conn_destroy(struct pn_direct_conn *direct_conn)

if (direct_conn->nonce != NULL)
g_free(direct_conn->nonce);

- direct_conn->link->direct_conn = NULL;
+ pn_peer_link_set_directconn(direct_conn->link, NULL);

g_free(direct_conn);

diff --git a/cvr/pn_direct_conn.h b/cvr/pn_direct_conn.h
index 3d203da..38a5d63 100644
--- a/cvr/pn_direct_conn.h
+++ b/cvr/pn_direct_conn.h
@@ -44,7 +44,7 @@ struct pn_direct_conn {

int c;
struct _PurpleProxyConnectData *connect_data;
- PecanStream *stream;
+ PnStream *stream;
};

struct pn_direct_conn *pn_direct_conn_new(struct pn_peer_link *link);
diff --git a/cvr/pn_peer_link.c b/cvr/pn_peer_link.c

index 162c7db..f535a50 100644
--- a/cvr/pn_peer_link.c
+++ b/cvr/pn_peer_link.c
@@ -789,3 +789,16 @@ pn_peer_link_request_object(struct pn_peer_link *link,

g_free(msnobj_base64);
}
+
+struct pn_direct_conn *
+pn_peer_link_get_directconn(const struct pn_peer_link *link)
+{
+ return link->direct_conn;
+}
+
+void
+pn_peer_link_set_directconn(struct pn_peer_link *link,

+ struct pn_direct_conn *direct_conn)

+{
+ link->direct_conn = direct_conn;
+}

diff --git a/cvr/pn_peer_link.h b/cvr/pn_peer_link.h
index 66ccd67..a33e578 100644

--- a/cvr/pn_peer_link.h
+++ b/cvr/pn_peer_link.h
@@ -31,6 +31,8 @@ struct MsnMessage;

struct _PurpleXfer;

+struct pn_peer_link;
+

typedef void (*MsnSlpCb) (struct pn_peer_call *call, const guchar *data, gsize size);
typedef void (*MsnSlpEndCb) (struct pn_peer_call *call, struct MsnSession *session);

@@ -70,4 +72,8 @@ struct pn_peer_link *msn_session_find_peer_link(struct MsnSession *session,

struct pn_peer_link *msn_session_get_peer_link(struct MsnSession *session,
const char *username);

+struct pn_direct_conn *pn_peer_link_get_directconn(const struct pn_peer_link *link);
+void pn_peer_link_set_directconn(struct pn_peer_link *link,

+ struct pn_direct_conn *direct_conn);

+
#endif /* PN_PEERLINK_H */
diff --git a/cvr/pn_peer_msg.c b/cvr/pn_peer_msg.c

index 31bfb9a..38b7991 100644
--- a/cvr/pn_peer_msg.c
+++ b/cvr/pn_peer_msg.c
@@ -698,8 +698,9 @@ got_ok(struct pn_peer_call *call,

if (strcmp(type, "application/x-msnmsgr-sessionreqbody") == 0) {
#ifdef MSN_DIRECTCONN
- if (call->link->session->use_direct_conn &&
- call->type == PN_PEER_CALL_DC)
+
+ if (msn_session_get_bool(pn_peer_link_get_session(call->link),

+ "use_direct_conn"))

{
/* First let's try a DirectConnection. */

@@ -720,7 +721,7 @@ got_ok(struct pn_peer_call *call,

1.6.5.3

[Octavio Alvarez]

It wasn't actually working before.
---
cvr/pn_direct_conn.c | 6 ++++++
1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/cvr/pn_direct_conn.c b/cvr/pn_direct_conn.c
index 99d3f5c..b1a0770 100644
--- a/cvr/pn_direct_conn.c
+++ b/cvr/pn_direct_conn.c
@@ -196,6 +196,9 @@ pn_direct_conn_write(struct pn_direct_conn *direct_conn,
{
/* Let's write the data. */
status = pn_stream_write (direct_conn->stream, data, len, &tmp, NULL);
+
+ if (status == G_IO_STATUS_NORMAL)
+ pn_stream_flush (direct_conn->stream, NULL);
}

if (status == G_IO_STATUS_NORMAL)
@@ -302,6 +305,7 @@ read_cb(GIOChannel *source, GIOCondition condition, gpointer data)
{
pn_error ("failed to allocate memory for read");

+ pn_direct_conn_destroy(direct_conn);
return FALSE;
}

@@ -374,6 +378,8 @@ connect_cb(gpointer data, gint source, const gchar *error_message)

[Felipe Contreras]

On Sun, Dec 6, 2009 at 10:49 PM, Octavio Alvarez
<alva...@alvarezp.ods.org> wrote:
> On Fri, 04 Dec 2009 02:27:45 -0800, Felipe Contreras
> <felipe.c...@gmail.com> wrote:
>
>>> +struct pn_peer_link;
>>> +
>>
>> I don't think this is actually needed, and it's unrelated to this
>> patch, but it's a good change anyway. I think the same should be done
>> for other structures in the cvr subdir, can you write a separate patch
>> for that?
>
> Sure.
>
> In this case it is needed to build. Otherwise I would have to include
> the definition in the .h by any mean, which would make the struct
> public.

No. With that line you are defining 'struct pn_peer_link', but there's
no need for that because it's defined in many other header files, like
pn_direct_conn.h.

I tried to revert that change and it didn't change anything.

> Are you using any particular coding style?

It's not defined anywhere, you just need to follow the current code.
Essentially tabs=4 spaces, and should be expaneded. If you use vim you
can set it up with:
set shiftwidth=4
set expandtab
set cinoptions=(0,t0

--
Felipe Contreras

[Felipe Contreras]

On Mon, Dec 7, 2009 at 12:29 AM, Octavio Alvarez
<alva...@alvarezp.ods.org> wrote:
> Based on previous patches from Felipe Contreras.

The patch looks good, but it still doesn't compile:
cvr/pn_direct_conn.c: In function ‘pn_direct_conn_send_handshake’:
cvr/pn_direct_conn.c:55: error: dereferencing pointer to incomplete type
cvr/pn_direct_conn.c:73: error: dereferencing pointer to incomplete type
cvr/pn_direct_conn.c:74: error: dereferencing pointer to incomplete type
cvr/pn_direct_conn.c:75: error: dereferencing pointer to incomplete type
cvr/pn_direct_conn.c: In function ‘read_cb’:
cvr/pn_direct_conn.c:284: warning: format ‘%d’ expects type ‘int’, but
argument 6 has type ‘struct GIOChannel *’

Am I missing something?

--
Felipe Contreras

[Felipe Contreras]

On Mon, Dec 7, 2009 at 12:29 AM, Octavio Alvarez
<alva...@alvarezp.ods.org> wrote:

> It wasn't actually working before.

ACK.

Once the first patch is applied I'll apply this one too.

--
Felipe Contreras