From: Mikel Astiz <[email protected]>
This patch series changes the way transfers are created, as discussed in IRC. Instead of having the session API offer all common operations, the modules now use the transfer API directly, and then queue the transfer in the session using obc_session_queue().
The motivation behind this change is that the transfer-initiator module needs to have access to the created object, to be able to access some member variables. This is required for the D-Bus changes in the pipeline.
Another alternative to this patch series would be to simply return the transfer pointer in the session API for functions such as obc_session_get(). IMO this would actually be better.
Please review the changes with care (specially patch 6/6) since there is a lot of code rewrite.
Mikel Astiz (6):
client: Fix possible double free of params
client: Split internal obc_transfer_register()
client: Transfer API splits create and register
client: Buffer-passing changes in transfer API
client: Flip parameter order in transfer API
client: Create transfers in modules
client/ftp.c | 25 +++++++---
client/manager.c | 24 +++++++---
client/map.c | 18 ++++---
client/pbap.c | 30 +++++++-----
client/session.c | 130 +++++++++++++----------------------------------------
client/session.h | 17 ++-----
client/sync.c | 20 +++++---
client/transfer.c | 83 +++++++++++++++++----------------
client/transfer.h | 27 +++++-------
9 files changed, 163 insertions(+), 211 deletions(-)
--
1.7.7.6
Hi Mikel,
On Fri, May 4, 2012 at 3:39 PM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> This patch series changes the way transfers are created, as discussed in IRC. Instead of having the session API offer all common operations, the modules now use the transfer API directly, and then queue the transfer in the session using obc_session_queue().
>
> The motivation behind this change is that the transfer-initiator module needs to have access to the created object, to be able to access some member variables. This is required for the D-Bus changes in the pipeline.
>
> Another alternative to this patch series would be to simply return the transfer pointer in the session API for functions such as obc_session_get(). IMO this would actually be better.
>
> Please review the changes with care (specially patch 6/6) since there is a lot of code rewrite.
>
> Mikel Astiz (6):
> ?client: Fix possible double free of params
> ?client: Split internal obc_transfer_register()
> ?client: Transfer API splits create and register
> ?client: Buffer-passing changes in transfer API
> ?client: Flip parameter order in transfer API
> ?client: Create transfers in modules
>
> ?client/ftp.c ? ? ?| ? 25 +++++++---
> ?client/manager.c ?| ? 24 +++++++---
> ?client/map.c ? ? ?| ? 18 ++++---
> ?client/pbap.c ? ? | ? 30 +++++++-----
> ?client/session.c ?| ?130 +++++++++++++----------------------------------------
> ?client/session.h ?| ? 17 ++-----
> ?client/sync.c ? ? | ? 20 +++++---
> ?client/transfer.c | ? 83 +++++++++++++++++----------------
> ?client/transfer.h | ? 27 +++++-------
> ?9 files changed, 163 insertions(+), 211 deletions(-)
>
> --
> 1.7.7.6
All 6 patches are now upstream.
--
Luiz Augusto von Dentz
From: Mikel Astiz <[email protected]>
After this patch the modules are responsible for creating the transfers,
and these objects must be queued using the session API.
This way the transfer initiator has full access to the transfer object,
in case for example it wants to access some member variable.
For convenience reasons, the function obc_session_queue() is designed
such that the preceding transfer creation might have failed. This means
that the given transfer might be NULL and the error object could already
be set. In this case the function will just return 0.
---
client/ftp.c | 25 ++++++++++----
client/manager.c | 24 ++++++++++----
client/map.c | 18 ++++++----
client/pbap.c | 30 ++++++++++-------
client/session.c | 94 +++++++++++------------------------------------------
client/session.h | 17 ++-------
client/sync.c | 20 +++++++----
7 files changed, 98 insertions(+), 130 deletions(-)
diff --git a/client/ftp.c b/client/ftp.c
index 86c2a3c..15c56f5 100644
--- a/client/ftp.c
+++ b/client/ftp.c
@@ -258,11 +258,14 @@ static DBusMessage *list_folder(DBusConnection *connection,
{
struct ftp_data *ftp = user_data;
struct obc_session *session = ftp->session;
+ struct obc_transfer *transfer;
GError *err = NULL;
- obc_session_get(session, "x-obex/folder-listing", NULL, NULL,
- NULL, 0, list_folder_callback, message, &err);
- if (err != NULL) {
+ transfer = obc_transfer_get("x-obex/folder-listing", NULL, NULL,
+ NULL, 0, &err);
+
+ if (!obc_session_queue(session, transfer, list_folder_callback,
+ message, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
"org.openobex.Error.Failed",
"%s", err->message);
@@ -280,6 +283,7 @@ static DBusMessage *get_file(DBusConnection *connection,
{
struct ftp_data *ftp = user_data;
struct obc_session *session = ftp->session;
+ struct obc_transfer *transfer;
const char *target_file, *source_file;
GError *err = NULL;
@@ -290,9 +294,11 @@ static DBusMessage *get_file(DBusConnection *connection,
return g_dbus_create_error(message,
"org.openobex.Error.InvalidArguments", NULL);
- obc_session_get(session, NULL, source_file, target_file, NULL, 0,
- get_file_callback, message, &err);
- if (err != NULL) {
+ transfer = obc_transfer_get(NULL, source_file, target_file, NULL, 0,
+ &err);
+
+ if (!obc_session_queue(session, transfer, get_file_callback, message,
+ &err)) {
DBusMessage *reply = g_dbus_create_error(message,
"org.openobex.Error.Failed",
"%s", err->message);
@@ -310,6 +316,7 @@ static DBusMessage *put_file(DBusConnection *connection,
{
struct ftp_data *ftp = user_data;
struct obc_session *session = ftp->session;
+ struct obc_transfer *transfer;
gchar *sourcefile, *targetfile;
GError *err = NULL;
@@ -321,8 +328,10 @@ static DBusMessage *put_file(DBusConnection *connection,
"org.openobex.Error.InvalidArguments",
"Invalid arguments in method call");
- obc_session_send(session, sourcefile, targetfile, &err);
- if (err != NULL) {
+ transfer = obc_transfer_put(NULL, targetfile, sourcefile, NULL, 0,
+ NULL, 0, &err);
+
+ if (!obc_session_queue(session, transfer, NULL, NULL, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
"org.openobex.Error.Failed",
"%s", err->message);
diff --git a/client/manager.c b/client/manager.c
index a46519f..93ba1d7 100644
--- a/client/manager.c
+++ b/client/manager.c
@@ -113,8 +113,12 @@ static void create_callback(struct obc_session *session,
for (i = 0; i < data->files->len; i++) {
const gchar *filename = g_ptr_array_index(data->files, i);
gchar *basename = g_path_get_basename(filename);
+ struct obc_transfer *transfer;
- if (obc_session_send(session, filename, basename, NULL) == 0) {
+ transfer = obc_transfer_put(NULL, basename, filename, NULL, 0,
+ NULL, 0, NULL);
+
+ if (!obc_session_queue(session, transfer, NULL, NULL, NULL)) {
g_free(basename);
break;
}
@@ -278,6 +282,7 @@ static void pull_obc_session_callback(struct obc_session *session,
GError *err, void *user_data)
{
struct send_data *data = user_data;
+ struct obc_transfer *pull;
DBusMessage *reply;
GError *gerr = NULL;
@@ -288,9 +293,11 @@ static void pull_obc_session_callback(struct obc_session *session,
goto fail;
}
- obc_session_pull(session, "text/x-vcard", data->filename,
- pull_complete_callback, data, &gerr);
- if (gerr != NULL) {
+ pull = obc_transfer_get("text/x-vcard", NULL, data->filename, NULL, 0,
+ &gerr);
+
+ if (!obc_session_queue(session, pull, pull_complete_callback, data,
+ &gerr)) {
reply = g_dbus_create_error(data->message,
"org.openobex.Error.Failed",
"%s", gerr->message);
@@ -497,6 +504,7 @@ static void capability_obc_session_callback(struct obc_session *session,
GError *err, void *user_data)
{
struct send_data *data = user_data;
+ struct obc_transfer *pull;
DBusMessage *reply;
GError *gerr = NULL;
@@ -507,9 +515,11 @@ static void capability_obc_session_callback(struct obc_session *session,
goto fail;
}
- obc_session_pull(session, "x-obex/capability", NULL,
- capabilities_complete_callback, data, &gerr);
- if (gerr != NULL) {
+ pull = obc_transfer_get("x-obex/capability", NULL, data->filename,
+ NULL, 0, &gerr);
+
+ if (!obc_session_queue(session, pull, capabilities_complete_callback,
+ data, &gerr)) {
reply = g_dbus_create_error(data->message,
"org.openobex.Error.Failed",
"%s", gerr->message);
diff --git a/client/map.c b/client/map.c
index eb06834..3bb01a6 100644
--- a/client/map.c
+++ b/client/map.c
@@ -135,12 +135,13 @@ static DBusMessage *map_get_folder_listing(DBusConnection *connection,
DBusMessage *message, void *user_data)
{
struct map_data *map = user_data;
+ struct obc_transfer *transfer;
GError *err = NULL;
- obc_session_get(map->session, "x-obex/folder-listing", NULL,
- NULL, NULL, 0,
- buffer_cb, map, &err);
- if (err != NULL) {
+ transfer = obc_transfer_get("x-obex/folder-listing", NULL, NULL,
+ NULL, 0, &err);
+
+ if (!obc_session_queue(map->session, transfer, buffer_cb, map, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
"org.openobex.Error.Failed",
"%s", err->message);
@@ -157,6 +158,7 @@ static DBusMessage *map_get_message_listing(DBusConnection *connection,
DBusMessage *message, void *user_data)
{
struct map_data *map = user_data;
+ struct obc_transfer *transfer;
const char *folder;
DBusMessageIter msg_iter;
GError *err = NULL;
@@ -169,10 +171,10 @@ static DBusMessage *map_get_message_listing(DBusConnection *connection,
dbus_message_iter_get_basic(&msg_iter, &folder);
- obc_session_get(map->session, "x-bt/MAP-msg-listing", folder,
- NULL, NULL, 0,
- buffer_cb, map, &err);
- if (err != NULL) {
+ transfer = obc_transfer_get("x-bt/MAP-msg-listing", folder, NULL,
+ NULL, 0, &err);
+
+ if (!obc_session_queue(map->session, transfer, buffer_cb, map, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
"org.openobex.Error.Failed",
"%s", err->message);
diff --git a/client/pbap.c b/client/pbap.c
index b178aff..33c6371 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -462,6 +462,7 @@ static DBusMessage *pull_phonebook(struct pbap_data *pbap,
{
struct pending_request *request;
struct pullphonebook_apparam apparam;
+ struct obc_transfer *transfer;
session_callback_t func;
GError *err = NULL;
@@ -492,10 +493,10 @@ static DBusMessage *pull_phonebook(struct pbap_data *pbap,
request = pending_request_new(pbap, message);
- obc_session_get(pbap->session, "x-bt/phonebook", name, NULL,
- (guint8 *) &apparam, sizeof(apparam),
- func, request, &err);
- if (err != NULL) {
+ transfer = obc_transfer_get("x-bt/phonebook", name, NULL, &apparam,
+ sizeof(apparam), &err);
+
+ if (!obc_session_queue(pbap->session, transfer, func, request, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
"org.openobex.Error.Failed",
"%s", err->message);
@@ -525,6 +526,7 @@ static DBusMessage *pull_vcard_listing(struct pbap_data *pbap,
guint16 count, guint16 offset)
{
struct pending_request *request;
+ struct obc_transfer *transfer;
guint8 *p, *apparam = NULL;
gint apparam_size;
GError *err = NULL;
@@ -557,11 +559,13 @@ static DBusMessage *pull_vcard_listing(struct pbap_data *pbap,
request = pending_request_new(pbap, message);
- obc_session_get(pbap->session, "x-bt/vcard-listing", name, NULL,
- apparam, apparam_size,
- pull_vcard_listing_callback, request, &err);
+ transfer = obc_transfer_get("x-bt/vcard-listing", name, NULL,
+ apparam, apparam_size, &err);
+
g_free(apparam);
- if (err != NULL) {
+
+ if (!obc_session_queue(pbap->session, transfer,
+ pull_vcard_listing_callback, request, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
"org.openobex.Error.Failed",
"%s", err->message);
@@ -760,6 +764,7 @@ static DBusMessage *pbap_pull_vcard(DBusConnection *connection,
struct pullvcardentry_apparam apparam;
const char *name;
struct pending_request *request;
+ struct obc_transfer *transfer;
GError *err = NULL;
if (!pbap->path)
@@ -782,10 +787,11 @@ static DBusMessage *pbap_pull_vcard(DBusConnection *connection,
request = pending_request_new(pbap, message);
- obc_session_get(pbap->session, "x-bt/vcard", name, NULL,
- (guint8 *)&apparam, sizeof(apparam),
- pull_phonebook_callback, request, &err);
- if (err != NULL) {
+ transfer = obc_transfer_get("x-bt/vcard", name, NULL,
+ &apparam, sizeof(apparam), &err);
+
+ if (!obc_session_queue(pbap->session, transfer, pull_phonebook_callback,
+ request, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
"org.openobex.Error.Failed",
"%s", err->message);
diff --git a/client/session.c b/client/session.c
index f8f8da4..f38f032 100644
--- a/client/session.c
+++ b/client/session.c
@@ -745,15 +745,30 @@ static int pending_request_auth(struct pending_request *p)
NULL);
}
-static guint session_request(struct obc_session *session,
- struct obc_transfer *transfer,
- session_callback_t func,
- void *data, GError **err)
+guint obc_session_queue(struct obc_session *session,
+ struct obc_transfer *transfer,
+ session_callback_t func, void *user_data,
+ GError **err)
{
struct pending_request *p;
const char *agent;
int perr;
+ if (transfer == NULL) {
+ if (err != NULL && *err == NULL)
+ g_set_error(err, OBEX_IO_ERROR, -EINVAL,
+ "Invalid arguments");
+
+ return 0;
+ }
+
+ if (session->obex == NULL) {
+ obc_transfer_unregister(transfer);
+ g_set_error(err, OBEX_IO_ERROR, -ENOTCONN,
+ "Session not connected");
+ return 0;
+ }
+
if (session->agent)
agent = obc_agent_get_name(session->agent);
else
@@ -767,7 +782,7 @@ static guint session_request(struct obc_session *session,
obc_transfer_set_callback(transfer, transfer_progress, session);
p = pending_request_new(session, transfer, session_start_transfer,
- func, data);
+ func, user_data);
if (session->p) {
g_queue_push_tail(session->queue, p);
return p->id;
@@ -956,56 +971,6 @@ static void session_start_transfer(gpointer data, gpointer user_data)
DBG("Transfer(%p) started", transfer);
}
-guint obc_session_get(struct obc_session *session, const char *type,
- const char *name, const char *targetfile,
- const guint8 *apparam, gint apparam_size,
- session_callback_t func, void *user_data,
- GError **err)
-{
- struct obc_transfer *transfer;
-
- if (session->obex == NULL) {
- g_set_error(err, OBEX_IO_ERROR, -ENOTCONN,
- "Session not connected");
- return 0;
- }
-
- transfer = obc_transfer_get(type, name, targetfile, apparam,
- apparam_size, err);
- if (transfer == NULL)
- return 0;
-
- return session_request(session, transfer, func, user_data, err);
-}
-
-guint obc_session_send(struct obc_session *session, const char *filename,
- const char *name, GError **err)
-{
- struct obc_transfer *transfer;
-
- if (session->obex == NULL) {
- g_set_error(err, OBEX_IO_ERROR, -ENOTCONN,
- "Session not connected");
- return 0;
- }
-
- transfer = obc_transfer_put(NULL, name, filename, NULL, 0, NULL, 0,
- err);
- if (transfer == NULL)
- return 0;
-
- return session_request(session, transfer, NULL, NULL, err);
-}
-
-guint obc_session_pull(struct obc_session *session,
- const char *type, const char *targetfile,
- session_callback_t function, void *user_data,
- GError **err)
-{
- return obc_session_get(session, type, NULL, targetfile, NULL, 0,
- function, user_data, err);
-}
-
const char *obc_session_register(struct obc_session *session,
GDBusDestroyFunction destroy)
{
@@ -1036,25 +1001,6 @@ fail:
return NULL;
}
-guint obc_session_put(struct obc_session *session, const char *contents,
- size_t size, const char *name, GError **err)
-{
- struct obc_transfer *transfer;
-
- if (session->obex == NULL) {
- g_set_error(err, OBEX_IO_ERROR, -ENOTCONN,
- "Session not connected");
- return 0;
- }
-
- transfer = obc_transfer_put(NULL, name, NULL, contents, size, NULL, 0,
- err);
- if (transfer == NULL)
- return 0;
-
- return session_request(session, transfer, NULL, NULL, err);
-}
-
static void agent_destroy(gpointer data, gpointer user_data)
{
struct obc_session *session = user_data;
diff --git a/client/session.h b/client/session.h
index 06aaa1b..2cac032 100644
--- a/client/session.h
+++ b/client/session.h
@@ -54,22 +54,13 @@ const char *obc_session_get_agent(struct obc_session *session);
const char *obc_session_get_path(struct obc_session *session);
const char *obc_session_get_target(struct obc_session *session);
-guint obc_session_send(struct obc_session *session, const char *filename,
- const char *name, GError **err);
-guint obc_session_get(struct obc_session *session, const char *type,
- const char *name, const char *targetfile,
- const guint8 *apparam, gint apparam_size,
- session_callback_t func, void *user_data,
- GError **err);
-guint obc_session_pull(struct obc_session *session,
- const char *type, const char *targetfile,
- session_callback_t function, void *user_data,
- GError **err);
const char *obc_session_register(struct obc_session *session,
GDBusDestroyFunction destroy);
-guint obc_session_put(struct obc_session *session, const char *contents,
- size_t size, const char *name, GError **err);
+guint obc_session_queue(struct obc_session *session,
+ struct obc_transfer *transfer,
+ session_callback_t func, void *user_data,
+ GError **err);
guint obc_session_setpath(struct obc_session *session, const char *path,
session_callback_t func, void *user_data,
GError **err);
diff --git a/client/sync.c b/client/sync.c
index c0b3800..278c405 100644
--- a/client/sync.c
+++ b/client/sync.c
@@ -127,6 +127,7 @@ static DBusMessage *sync_getphonebook(DBusConnection *connection,
DBusMessage *message, void *user_data)
{
struct sync_data *sync = user_data;
+ struct obc_transfer *transfer;
GError *err = NULL;
if (sync->msg)
@@ -137,11 +138,12 @@ static DBusMessage *sync_getphonebook(DBusConnection *connection,
if (!sync->phonebook_path)
sync->phonebook_path = g_strdup("telecom/pb.vcf");
- obc_session_get(sync->session, "phonebook", sync->phonebook_path,
- NULL, NULL, 0,
- sync_getphonebook_callback, sync,
- &err);
- if (err != 0) {
+ transfer = obc_transfer_get("phonebook", sync->phonebook_path,
+ NULL, NULL, 0, &err);
+
+ if (!obc_session_queue(sync->session, transfer,
+ sync_getphonebook_callback,
+ sync, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
ERROR_INF ".Failed",
err->message);
@@ -158,6 +160,7 @@ static DBusMessage *sync_putphonebook(DBusConnection *connection,
DBusMessage *message, void *user_data)
{
struct sync_data *sync = user_data;
+ struct obc_transfer *transfer;
const char *buf;
GError *err = NULL;
@@ -171,9 +174,10 @@ static DBusMessage *sync_putphonebook(DBusConnection *connection,
if (!sync->phonebook_path)
sync->phonebook_path = g_strdup("telecom/pb.vcf");
- obc_session_put(sync->session, buf, strlen(buf), sync->phonebook_path,
- &err);
- if (err != NULL) {
+ transfer = obc_transfer_put(NULL, sync->phonebook_path, NULL, buf,
+ strlen(buf), NULL, 0, &err);
+
+ if (!obc_session_queue(sync->session, transfer, NULL, NULL, &err)) {
DBusMessage *reply = g_dbus_create_error(message,
ERROR_INF ".Failed",
err->message);
--
1.7.7.6
From: Mikel Astiz <[email protected]>
This minor change makes the transfer API more consistent with the
parameter-order used in the session API.
---
client/session.c | 4 ++--
client/transfer.c | 8 ++++----
client/transfer.h | 8 ++++----
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/client/session.c b/client/session.c
index 8523a86..f8f8da4 100644
--- a/client/session.c
+++ b/client/session.c
@@ -970,7 +970,7 @@ guint obc_session_get(struct obc_session *session, const char *type,
return 0;
}
- transfer = obc_transfer_get(targetfile, name, type, apparam,
+ transfer = obc_transfer_get(type, name, targetfile, apparam,
apparam_size, err);
if (transfer == NULL)
return 0;
@@ -989,7 +989,7 @@ guint obc_session_send(struct obc_session *session, const char *filename,
return 0;
}
- transfer = obc_transfer_put(filename, name, NULL, NULL, 0, NULL, 0,
+ transfer = obc_transfer_put(NULL, name, filename, NULL, 0, NULL, 0,
err);
if (transfer == NULL)
return 0;
diff --git a/client/transfer.c b/client/transfer.c
index 7bf687c..ecf3232 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -319,9 +319,9 @@ done:
return TRUE;
}
-struct obc_transfer *obc_transfer_get(const char *filename,
+struct obc_transfer *obc_transfer_get(const char *type,
const char *name,
- const char *type,
+ const char *filename,
const void *params, size_t psize,
GError **err)
{
@@ -340,9 +340,9 @@ struct obc_transfer *obc_transfer_get(const char *filename,
return transfer;
}
-struct obc_transfer *obc_transfer_put(const char *filename,
+struct obc_transfer *obc_transfer_put(const char *type,
const char *name,
- const char *type,
+ const char *filename,
const void *contents, size_t csize,
const void *params, size_t psize,
GError **err)
diff --git a/client/transfer.h b/client/transfer.h
index aebba7f..94bb800 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -27,14 +27,14 @@ typedef void (*transfer_callback_t) (struct obc_transfer *transfer,
gint64 transferred, GError *err,
void *user_data);
-struct obc_transfer *obc_transfer_get(const char *filename,
+struct obc_transfer *obc_transfer_get(const char *type,
const char *name,
- const char *type,
+ const char *filename,
const void *params, size_t psize,
GError **err);
-struct obc_transfer *obc_transfer_put(const char *filename,
+struct obc_transfer *obc_transfer_put(const char *type,
const char *name,
- const char *type,
+ const char *filename,
const void *contents, size_t csize,
const void *params, size_t psize,
GError **err);
--
1.7.7.6
From: Mikel Astiz <[email protected]>
The creation process has been internally split into two steps: creation
and D-Bus registration. This is easier to understand and it also allows
to expose these two-steps in the transfer API.
---
client/transfer.c | 51 ++++++++++++++++++++++++++++-----------------------
1 files changed, 28 insertions(+), 23 deletions(-)
diff --git a/client/transfer.c b/client/transfer.c
index 58baa85..a725e19 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -223,29 +223,35 @@ static void obc_transfer_free(struct obc_transfer *transfer)
g_free(transfer);
}
-static struct obc_transfer *obc_transfer_register(DBusConnection *conn,
- const char *agent,
- guint8 op,
+static struct obc_transfer *obc_transfer_create(guint8 op,
const char *filename,
const char *name,
- const char *type,
- GError **err)
+ const char *type)
{
struct obc_transfer *transfer;
transfer = g_new0(struct obc_transfer, 1);
transfer->op = op;
- transfer->agent = g_strdup(agent);
transfer->filename = g_strdup(filename);
transfer->name = g_strdup(name);
transfer->type = g_strdup(type);
+ return transfer;
+}
+
+static gboolean obc_transfer_register(struct obc_transfer *transfer,
+ DBusConnection *conn,
+ const char *agent,
+ GError **err)
+{
/* for OBEX specific mime types we don't need to register a transfer */
- if (type != NULL &&
- (strncmp(type, "x-obex/", 7) == 0 ||
- strncmp(type, "x-bt/", 5) == 0))
+ if (transfer->type != NULL &&
+ (strncmp(transfer->type, "x-obex/", 7) == 0 ||
+ strncmp(transfer->type, "x-bt/", 5) == 0))
goto done;
+ transfer->agent = g_strdup(agent);
+
transfer->path = g_strdup_printf("%s/transfer%ju",
TRANSFER_BASEPATH, counter++);
@@ -253,7 +259,7 @@ static struct obc_transfer *obc_transfer_register(DBusConnection *conn,
if (transfer->conn == NULL) {
g_set_error(err, OBC_TRANSFER_ERROR, -EFAULT,
"Unable to connect to D-Bus");
- goto fail;
+ return FALSE;
}
if (g_dbus_register_interface(transfer->conn, transfer->path,
@@ -262,18 +268,13 @@ static struct obc_transfer *obc_transfer_register(DBusConnection *conn,
transfer, NULL) == FALSE) {
g_set_error(err, OBC_TRANSFER_ERROR, -EFAULT,
"Unable to register to D-Bus");
- goto fail;
+ return FALSE;
}
done:
DBG("%p registered %s", transfer, transfer->path);
- return transfer;
-
-fail:
- obc_transfer_free(transfer);
-
- return NULL;
+ return TRUE;
}
static gboolean transfer_open(struct obc_transfer *transfer, int flags,
@@ -316,10 +317,12 @@ struct obc_transfer *obc_transfer_get(DBusConnection *conn,
struct obc_transfer *transfer;
int perr;
- transfer = obc_transfer_register(conn, agent, G_OBEX_OP_GET, filename,
- name, type, err);
- if (transfer == NULL)
+ transfer = obc_transfer_create(G_OBEX_OP_GET, filename, name, type);
+
+ if (!obc_transfer_register(transfer, conn, agent, err)) {
+ obc_transfer_free(transfer);
return NULL;
+ }
perr = transfer_open(transfer, O_WRONLY | O_CREAT | O_TRUNC, 0600, err);
if (perr < 0) {
@@ -346,10 +349,12 @@ struct obc_transfer *obc_transfer_put(DBusConnection *conn,
struct stat st;
int perr;
- transfer = obc_transfer_register(conn, agent, G_OBEX_OP_PUT, filename,
- name, type, err);
- if (transfer == NULL)
+ transfer = obc_transfer_create(G_OBEX_OP_PUT, filename, name, type);
+
+ if (!obc_transfer_register(transfer, conn, agent, err)) {
+ obc_transfer_free(transfer);
return NULL;
+ }
if (contents != NULL) {
ssize_t w;
--
1.7.7.6
From: Mikel Astiz <[email protected]>
Transfer API now takes const buffers (both params and contents) and
internally copies the memory as necessary. This new API is safer to use,
which is convenient if the modules would start using it directly.
---
client/session.c | 23 ++++++-----------------
client/transfer.c | 35 +++++++++++++++++++++++------------
client/transfer.h | 12 +++---------
3 files changed, 32 insertions(+), 38 deletions(-)
diff --git a/client/session.c b/client/session.c
index e9993c8..8523a86 100644
--- a/client/session.c
+++ b/client/session.c
@@ -963,7 +963,6 @@ guint obc_session_get(struct obc_session *session, const char *type,
GError **err)
{
struct obc_transfer *transfer;
- struct obc_transfer_params *params = NULL;
if (session->obex == NULL) {
g_set_error(err, OBEX_IO_ERROR, -ENOTCONN,
@@ -971,21 +970,10 @@ guint obc_session_get(struct obc_session *session, const char *type,
return 0;
}
- if (apparam != NULL) {
- params = g_new0(struct obc_transfer_params, 1);
- params->data = g_new(guint8, apparam_size);
- memcpy(params->data, apparam, apparam_size);
- params->size = apparam_size;
- }
-
- transfer = obc_transfer_get(targetfile, name, type, params, err);
- if (transfer == NULL) {
- if (params != NULL) {
- g_free(params->data);
- g_free(params);
- }
+ transfer = obc_transfer_get(targetfile, name, type, apparam,
+ apparam_size, err);
+ if (transfer == NULL)
return 0;
- }
return session_request(session, transfer, func, user_data, err);
}
@@ -1001,7 +989,8 @@ guint obc_session_send(struct obc_session *session, const char *filename,
return 0;
}
- transfer = obc_transfer_put(filename, name, NULL, NULL, 0, NULL, err);
+ transfer = obc_transfer_put(filename, name, NULL, NULL, 0, NULL, 0,
+ err);
if (transfer == NULL)
return 0;
@@ -1058,7 +1047,7 @@ guint obc_session_put(struct obc_session *session, const char *contents,
return 0;
}
- transfer = obc_transfer_put(NULL, name, NULL, contents, size, NULL,
+ transfer = obc_transfer_put(NULL, name, NULL, contents, size, NULL, 0,
err);
if (transfer == NULL)
return 0;
diff --git a/client/transfer.c b/client/transfer.c
index 7a55fe6..7bf687c 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -53,6 +53,11 @@ struct transfer_callback {
void *data;
};
+struct obc_transfer_params {
+ void *data;
+ size_t size;
+};
+
struct obc_transfer {
GObex *obex;
guint8 op;
@@ -226,7 +231,9 @@ static void obc_transfer_free(struct obc_transfer *transfer)
static struct obc_transfer *obc_transfer_create(guint8 op,
const char *filename,
const char *name,
- const char *type)
+ const char *type,
+ const void *params,
+ size_t psize)
{
struct obc_transfer *transfer;
@@ -236,6 +243,12 @@ static struct obc_transfer *obc_transfer_create(guint8 op,
transfer->name = g_strdup(name);
transfer->type = g_strdup(type);
+ if (params != NULL) {
+ transfer->params = g_new0(struct obc_transfer_params, 1);
+ transfer->params->data = g_memdup(params, psize);
+ transfer->params->size = psize;
+ }
+
return transfer;
}
@@ -309,13 +322,14 @@ done:
struct obc_transfer *obc_transfer_get(const char *filename,
const char *name,
const char *type,
- struct obc_transfer_params *params,
+ const void *params, size_t psize,
GError **err)
{
struct obc_transfer *transfer;
int perr;
- transfer = obc_transfer_create(G_OBEX_OP_GET, filename, name, type);
+ transfer = obc_transfer_create(G_OBEX_OP_GET, filename, name, type,
+ params, psize);
perr = transfer_open(transfer, O_WRONLY | O_CREAT | O_TRUNC, 0600, err);
if (perr < 0) {
@@ -323,24 +337,22 @@ struct obc_transfer *obc_transfer_get(const char *filename,
return NULL;
}
- transfer->params = params;
-
return transfer;
}
struct obc_transfer *obc_transfer_put(const char *filename,
const char *name,
const char *type,
- const char *contents,
- size_t size,
- struct obc_transfer_params *params,
+ const void *contents, size_t csize,
+ const void *params, size_t psize,
GError **err)
{
struct obc_transfer *transfer;
struct stat st;
int perr;
- transfer = obc_transfer_create(G_OBEX_OP_PUT, filename, name, type);
+ transfer = obc_transfer_create(G_OBEX_OP_PUT, filename, name, type,
+ params, psize);
if (contents != NULL) {
ssize_t w;
@@ -348,12 +360,12 @@ struct obc_transfer *obc_transfer_put(const char *filename,
if (!transfer_open(transfer, O_RDWR, 0, err))
goto fail;
- w = write(transfer->fd, contents, size);
+ w = write(transfer->fd, contents, csize);
if (w < 0) {
error("write(): %s(%d)", strerror(errno), errno);
perr = -errno;
goto fail;
- } else if ((size_t) w != size) {
+ } else if ((size_t) w != csize) {
error("Unable to write all contents to file");
perr = -EFAULT;
goto fail;
@@ -372,7 +384,6 @@ struct obc_transfer *obc_transfer_put(const char *filename,
}
transfer->size = st.st_size;
- transfer->params = params;
return transfer;
diff --git a/client/transfer.h b/client/transfer.h
index 073b279..aebba7f 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -21,11 +21,6 @@
*
*/
-struct obc_transfer_params {
- void *data;
- size_t size;
-};
-
struct obc_transfer;
typedef void (*transfer_callback_t) (struct obc_transfer *transfer,
@@ -35,14 +30,13 @@ typedef void (*transfer_callback_t) (struct obc_transfer *transfer,
struct obc_transfer *obc_transfer_get(const char *filename,
const char *name,
const char *type,
- struct obc_transfer_params *params,
+ const void *params, size_t psize,
GError **err);
struct obc_transfer *obc_transfer_put(const char *filename,
const char *name,
const char *type,
- const char *contents,
- size_t size,
- struct obc_transfer_params *params,
+ const void *contents, size_t csize,
+ const void *params, size_t psize,
GError **err);
gboolean obc_transfer_register(struct obc_transfer *transfer,
--
1.7.7.6
From: Mikel Astiz <[email protected]>
The transfer-creating functions (obc_transfer_get and obc_transfer_put)
no longer register the transfer automatically.
This separation makes it possible that the modules would create the
transfers and then pass the object to the session, which would be
responsible for the registration.
---
client/session.c | 33 +++++++++++++++------------------
client/transfer.c | 20 +++-----------------
client/transfer.h | 13 +++++++------
3 files changed, 25 insertions(+), 41 deletions(-)
diff --git a/client/session.c b/client/session.c
index 3a8807b..e9993c8 100644
--- a/client/session.c
+++ b/client/session.c
@@ -751,8 +751,19 @@ static guint session_request(struct obc_session *session,
void *data, GError **err)
{
struct pending_request *p;
+ const char *agent;
int perr;
+ if (session->agent)
+ agent = obc_agent_get_name(session->agent);
+ else
+ agent = NULL;
+
+ if (!obc_transfer_register(transfer, session->conn, agent, err)) {
+ obc_transfer_unregister(transfer);
+ return 0;
+ }
+
obc_transfer_set_callback(transfer, transfer_progress, session);
p = pending_request_new(session, transfer, session_start_transfer,
@@ -953,7 +964,6 @@ guint obc_session_get(struct obc_session *session, const char *type,
{
struct obc_transfer *transfer;
struct obc_transfer_params *params = NULL;
- const char *agent;
if (session->obex == NULL) {
g_set_error(err, OBEX_IO_ERROR, -ENOTCONN,
@@ -968,13 +978,7 @@ guint obc_session_get(struct obc_session *session, const char *type,
params->size = apparam_size;
}
- if (session->agent)
- agent = obc_agent_get_name(session->agent);
- else
- agent = NULL;
-
- transfer = obc_transfer_get(session->conn, agent, targetfile, name,
- type, params, err);
+ transfer = obc_transfer_get(targetfile, name, type, params, err);
if (transfer == NULL) {
if (params != NULL) {
g_free(params->data);
@@ -990,7 +994,6 @@ guint obc_session_send(struct obc_session *session, const char *filename,
const char *name, GError **err)
{
struct obc_transfer *transfer;
- const char *agent;
if (session->obex == NULL) {
g_set_error(err, OBEX_IO_ERROR, -ENOTCONN,
@@ -998,10 +1001,7 @@ guint obc_session_send(struct obc_session *session, const char *filename,
return 0;
}
- agent = obc_agent_get_name(session->agent);
-
- transfer = obc_transfer_put(session->conn, agent, filename, name,
- NULL, NULL, 0, NULL, err);
+ transfer = obc_transfer_put(filename, name, NULL, NULL, 0, NULL, err);
if (transfer == NULL)
return 0;
@@ -1051,7 +1051,6 @@ guint obc_session_put(struct obc_session *session, const char *contents,
size_t size, const char *name, GError **err)
{
struct obc_transfer *transfer;
- const char *agent;
if (session->obex == NULL) {
g_set_error(err, OBEX_IO_ERROR, -ENOTCONN,
@@ -1059,10 +1058,8 @@ guint obc_session_put(struct obc_session *session, const char *contents,
return 0;
}
- agent = obc_agent_get_name(session->agent);
-
- transfer = obc_transfer_put(session->conn, agent, NULL, name, NULL,
- contents, size, NULL, err);
+ transfer = obc_transfer_put(NULL, name, NULL, contents, size, NULL,
+ err);
if (transfer == NULL)
return 0;
diff --git a/client/transfer.c b/client/transfer.c
index a725e19..7a55fe6 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -239,7 +239,7 @@ static struct obc_transfer *obc_transfer_create(guint8 op,
return transfer;
}
-static gboolean obc_transfer_register(struct obc_transfer *transfer,
+gboolean obc_transfer_register(struct obc_transfer *transfer,
DBusConnection *conn,
const char *agent,
GError **err)
@@ -306,9 +306,7 @@ done:
return TRUE;
}
-struct obc_transfer *obc_transfer_get(DBusConnection *conn,
- const char *agent,
- const char *filename,
+struct obc_transfer *obc_transfer_get(const char *filename,
const char *name,
const char *type,
struct obc_transfer_params *params,
@@ -319,11 +317,6 @@ struct obc_transfer *obc_transfer_get(DBusConnection *conn,
transfer = obc_transfer_create(G_OBEX_OP_GET, filename, name, type);
- if (!obc_transfer_register(transfer, conn, agent, err)) {
- obc_transfer_free(transfer);
- return NULL;
- }
-
perr = transfer_open(transfer, O_WRONLY | O_CREAT | O_TRUNC, 0600, err);
if (perr < 0) {
obc_transfer_free(transfer);
@@ -335,9 +328,7 @@ struct obc_transfer *obc_transfer_get(DBusConnection *conn,
return transfer;
}
-struct obc_transfer *obc_transfer_put(DBusConnection *conn,
- const char *agent,
- const char *filename,
+struct obc_transfer *obc_transfer_put(const char *filename,
const char *name,
const char *type,
const char *contents,
@@ -351,11 +342,6 @@ struct obc_transfer *obc_transfer_put(DBusConnection *conn,
transfer = obc_transfer_create(G_OBEX_OP_PUT, filename, name, type);
- if (!obc_transfer_register(transfer, conn, agent, err)) {
- obc_transfer_free(transfer);
- return NULL;
- }
-
if (contents != NULL) {
ssize_t w;
diff --git a/client/transfer.h b/client/transfer.h
index 1b83d18..073b279 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -32,16 +32,12 @@ typedef void (*transfer_callback_t) (struct obc_transfer *transfer,
gint64 transferred, GError *err,
void *user_data);
-struct obc_transfer *obc_transfer_get(DBusConnection *conn,
- const char *agent,
- const char *filename,
+struct obc_transfer *obc_transfer_get(const char *filename,
const char *name,
const char *type,
struct obc_transfer_params *params,
GError **err);
-struct obc_transfer *obc_transfer_put(DBusConnection *conn,
- const char *agent,
- const char *filename,
+struct obc_transfer *obc_transfer_put(const char *filename,
const char *name,
const char *type,
const char *contents,
@@ -49,6 +45,11 @@ struct obc_transfer *obc_transfer_put(DBusConnection *conn,
struct obc_transfer_params *params,
GError **err);
+gboolean obc_transfer_register(struct obc_transfer *transfer,
+ DBusConnection *conn,
+ const char *agent,
+ GError **err);
+
void obc_transfer_unregister(struct obc_transfer *transfer);
gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
--
1.7.7.6
From: Mikel Astiz <[email protected]>
obc_transfer_get() and obc_transfer_put() should only assume ownership
of the given params only in case of success. Otherwise some erros might
result in a double free of such memory.
---
client/transfer.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/client/transfer.c b/client/transfer.c
index 411a7c0..58baa85 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -229,7 +229,6 @@ static struct obc_transfer *obc_transfer_register(DBusConnection *conn,
const char *filename,
const char *name,
const char *type,
- struct obc_transfer_params *params,
GError **err)
{
struct obc_transfer *transfer;
@@ -240,7 +239,6 @@ static struct obc_transfer *obc_transfer_register(DBusConnection *conn,
transfer->filename = g_strdup(filename);
transfer->name = g_strdup(name);
transfer->type = g_strdup(type);
- transfer->params = params;
/* for OBEX specific mime types we don't need to register a transfer */
if (type != NULL &&
@@ -319,7 +317,7 @@ struct obc_transfer *obc_transfer_get(DBusConnection *conn,
int perr;
transfer = obc_transfer_register(conn, agent, G_OBEX_OP_GET, filename,
- name, type, params, err);
+ name, type, err);
if (transfer == NULL)
return NULL;
@@ -329,6 +327,8 @@ struct obc_transfer *obc_transfer_get(DBusConnection *conn,
return NULL;
}
+ transfer->params = params;
+
return transfer;
}
@@ -347,7 +347,7 @@ struct obc_transfer *obc_transfer_put(DBusConnection *conn,
int perr;
transfer = obc_transfer_register(conn, agent, G_OBEX_OP_PUT, filename,
- name, type, params, err);
+ name, type, err);
if (transfer == NULL)
return NULL;
@@ -381,6 +381,7 @@ struct obc_transfer *obc_transfer_put(DBusConnection *conn,
}
transfer->size = st.st_size;
+ transfer->params = params;
return transfer;
--
1.7.7.6