2012-04-10 13:38:21

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 00/11] client: transfer implementation improvements

From: Mikel Astiz <[email protected]>

This second version of the series integrates the review from Luiz, affecting patches from v0 01/11 to 05/11. These changes mainly consist of minor corrections including early return statements and function renaming.

>From previous cover letter:

This patch series includes internal rewrites particularly regarding transfer.c. It includes some api changes, minor fixes and internal refactoring, none of which should affect the D-Bus api.

These changes are convenient by themselves, but they are also motivated by the D-Bus changes to be discussed soon. For example, patch 07/11 separates memory-based transfers from the filesystem-based ones, which will make it easier to decide if memory-based transfers should be removed (as it has been proposed).

Another example is patch 08/11, which will be get simplified once the D-Bus agent is removed.

Mikel Astiz (11):
client: minor buffer access api changes
client: remove unused field
client: refactor transfer callback use
client: fix progress report before field update
client: transfer api merges put and get
client: transfers take gobex when starting
client: separate memory transfers in transfer api
client: open transfer file during creation
client: delete file in case of error
client: unify transfer implementation
client: double transfer buf size in each realloc

client/pbap.c | 2 +-
client/session.c | 146 +++++-------
client/session.h | 4 +-
client/transfer.c | 683 ++++++++++++++++++++++++++++++++---------------------
client/transfer.h | 48 +++--
5 files changed, 508 insertions(+), 375 deletions(-)

--
1.7.7.6



2012-04-12 11:08:55

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd v1 04/11] client: fix progress report before field update

Hi Mikel,

On Tue, Apr 10, 2012 at 4:38 PM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> The progress report in put_xfer_progress was done before the
> corresponding field was updated.
> ---
> ?client/transfer.c | ? ?4 ++--
> ?1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/client/transfer.c b/client/transfer.c
> index 3b4f8f2..b558d84 100644
> --- a/client/transfer.c
> +++ b/client/transfer.c
> @@ -494,10 +494,10 @@ static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
> ? ? ? ?if (size <= 0)
> ? ? ? ? ? ? ? ?return size;
>
> - ? ? ? transfer_notify_progress(transfer);
> -
> ? ? ? ?transfer->transferred += size;
>
> + ? ? ? transfer_notify_progress(transfer);
> +
> ? ? ? ?return size;
> ?}
>
> --
> 1.7.7.6

git blame indicates this has been there in the past but it was changed
by this commit:

commit e1891acc03a22cc59f0dfb29869243f3ca81e716
Author: Luiz Augusto von Dentz <[email protected]>
Date: Mon Oct 3 18:43:16 2011 +0300

Fix possible crash when indicating progress

The amount of transferred bytes should only be updated after it has
been sent.

It may not crash anymore, but the change is still logically correct
since the data has yet to be sent.

--
Luiz Augusto von Dentz

2012-04-12 10:58:53

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd v1 03/11] client: refactor transfer callback use

Hi Mikel,

On Tue, Apr 10, 2012 at 4:38 PM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> Separated internal functions are used to report success, failure, and
> progress, instead of using the callback directly.
>
> This makes the code more readable.

It adds more debug output but its also adds more lines of code,
perhaps if you join transfer_notify_complete and transfer_notify_error
then Im fine with it.

--
Luiz Augusto von Dentz

2012-04-12 10:31:27

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd v1 02/11] client: remove unused field

Hi Mikel,

On Tue, Apr 10, 2012 at 4:38 PM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> The errorcode field is set but never used, so it can safely be removed.
> In addition there is no need for such a field, because errors can be
> propagated using the available callback.
> ---
> ?client/transfer.c | ? 22 +++++-----------------
> ?1 files changed, 5 insertions(+), 17 deletions(-)
>
> diff --git a/client/transfer.c b/client/transfer.c
> index 988ec10..a609aa9 100644
> --- a/client/transfer.c
> +++ b/client/transfer.c
> @@ -70,7 +70,6 @@ struct obc_transfer {
> ? ? ? ?int filled;
> ? ? ? ?gint64 size;
> ? ? ? ?gint64 transferred;
> - ? ? ? int err;
> ?};
>
> ?static GQuark obc_transfer_error_quark(void)
> @@ -310,10 +309,8 @@ static void get_buf_xfer_complete(GObex *obex, GError *err, gpointer user_data)
>
> ? ? ? ?transfer->xfer = 0;
>
> - ? ? ? if (err) {
> - ? ? ? ? ? ? ? transfer->err = err->code;
> + ? ? ? if (err)
> ? ? ? ? ? ? ? ?goto done;
> - ? ? ? }
>
> ? ? ? ?if (transfer->filled > 0 &&
> ? ? ? ? ? ? ? ? ? ? ? ?transfer->buffer[transfer->filled - 1] == '\0')
> @@ -407,14 +404,9 @@ static void xfer_complete(GObex *obex, GError *err, gpointer user_data)
>
> ? ? ? ?transfer->xfer = 0;
>
> - ? ? ? if (err) {
> - ? ? ? ? ? ? ? transfer->err = err->code;
> - ? ? ? ? ? ? ? goto done;
> - ? ? ? }
> -
> - ? ? ? transfer->size = transfer->transferred;
> + ? ? ? if (err == NULL)
> + ? ? ? ? ? ? ? transfer->size = transfer->transferred;
>
> -done:
> ? ? ? ?if (callback)
> ? ? ? ? ? ? ? ?callback->func(transfer, transfer->size, err, callback->data);
> ?}
> @@ -431,10 +423,8 @@ static gboolean get_xfer_progress(const void *buf, gsize len,
> ? ? ? ? ? ? ? ?gint w;
>
> ? ? ? ? ? ? ? ?w = write(transfer->fd, transfer->buffer, transfer->filled);
> - ? ? ? ? ? ? ? if (w < 0) {
> - ? ? ? ? ? ? ? ? ? ? ? transfer->err = -errno;
> + ? ? ? ? ? ? ? if (w < 0)
> ? ? ? ? ? ? ? ? ? ? ? ?return FALSE;
> - ? ? ? ? ? ? ? }
>
> ? ? ? ? ? ? ? ?transfer->filled -= w;
> ? ? ? ?}
> @@ -478,10 +468,8 @@ static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
> ? ? ? ?gssize size;
>
> ? ? ? ?size = read(transfer->fd, buf, len);
> - ? ? ? if (size <= 0) {
> - ? ? ? ? ? ? ? transfer->err = -errno;
> + ? ? ? if (size <= 0)
> ? ? ? ? ? ? ? ?return size;
> - ? ? ? }
>
> ? ? ? ?if (callback)
> ? ? ? ? ? ? ? ?callback->func(transfer, transfer->transferred, NULL,
> --
> 1.7.7.6

Ack.

--
Luiz Augusto von Dentz

2012-04-12 10:30:04

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd v1 01/11] client: minor buffer access api changes

Hi Mikel,

On Tue, Apr 10, 2012 at 4:38 PM, Mikel Astiz <[email protected]> wrote:
> @@ -1169,29 +1168,18 @@ const char *obc_session_get_buffer(struct obc_session *session, size_t *size)
> ? ? ? ? ? ? ? ?return NULL;
> ? ? ? ?}
>
> - ? ? ? buf = obc_transfer_get_buffer(transfer, size);
> -
> - ? ? ? obc_transfer_clear_buffer(transfer);
> -
> - ? ? ? return buf;
> + ? ? ? return obc_transfer_get_buffer(transfer, size);
> ?}
>

Lets just not mix unrelated things so leave clear_buffer for now,
besides I would really like to remove the memory buffering completely
and start using temporary files, so doing cosmetic changes IMO is
useless if we are not fixing real bugs.


--
Luiz Augusto von Dentz

2012-04-10 13:38:32

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 11/11] client: double transfer buf size in each realloc

From: Mikel Astiz <[email protected]>

This provides linear time complexity instead of quadratic time, which
can be considerable for big transfers.

The obvious drawback is that more memory will be required.
---
client/transfer.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index c7d74a9..7237db8 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -540,16 +540,17 @@ static gboolean handle_get_body(struct obc_transfer *transfer, GObexPacket *rsp)
}
} else {
struct mem_location *location = transfer->mem_location;
- gsize bsize;
+ gint64 req_size;

assert(location != NULL);

- /* copy all buffered data */
- bsize = location->buffer_len - transfer->transferred;
-
/* for convenience, leave space for final null character */
- if (bsize < len + 1) {
- location->buffer_len += len + 1 - bsize;
+ req_size = transfer->transferred + len + 1;
+
+ if (location->buffer_len < req_size) {
+ while (location->buffer_len < req_size)
+ location->buffer_len *= 2;
+
location->buffer = g_realloc(location->buffer,
location->buffer_len);
}
--
1.7.7.6


2012-04-10 13:38:29

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 08/11] client: open transfer file during creation

From: Mikel Astiz <[email protected]>

For file-based transfers, it is safer to open the file during the
creation of the transfer, instead of waiting until the transfer is
actually started.

This also simplifies the transfer api, by make obc_transfer_set_file
unnecessary.

This behavior is slightly more complicated when an agent is being used,
since it can overwrite the filename for get operations. This will only
be known once the transfer has been authorized, that is, when it's
starting.
---
client/session.c | 48 ++++++++++++-------
client/transfer.c | 137 ++++++++++++++++++++++++++++++++++------------------
client/transfer.h | 12 +++--
3 files changed, 126 insertions(+), 71 deletions(-)

diff --git a/client/session.c b/client/session.c
index 679627b..3abb342 100644
--- a/client/session.c
+++ b/client/session.c
@@ -688,8 +688,16 @@ static void session_request_reply(DBusPendingCall *call, gpointer user_data)
if (strlen(name)) {
if (obc_transfer_get_cmd(transfer) == OBC_TRANSFER_PUT)
obc_transfer_set_name(transfer, name);
- else
- obc_transfer_set_filename(transfer, name);
+ else {
+ GError *gerr = NULL;
+
+ if (!obc_transfer_set_filename(transfer, name, &gerr)) {
+ session_terminate_transfer(session, transfer,
+ gerr);
+ g_clear_error(&gerr);
+ return;
+ }
+ }
}

if (p->auth_complete)
@@ -938,6 +946,7 @@ int obc_session_get(struct obc_session *session, const char *type,
struct obc_transfer *transfer;
struct obc_transfer_params *params = NULL;
const char *agent;
+ GError *err = NULL;

if (session->obex == NULL)
return -ENOTCONN;
@@ -958,15 +967,19 @@ int obc_session_get(struct obc_session *session, const char *type,
transfer = obc_transfer_create(session->conn, agent,
OBC_TRANSFER_GET,
targetfile, name, type,
- params);
+ params, &err);
else
transfer = obc_transfer_create_mem(session->conn, agent,
OBC_TRANSFER_GET,
NULL, 0, NULL,
- name, type, params);
+ name, type, params,
+ &err);

- if (transfer == NULL)
- return -EIO;
+ if (transfer == NULL) {
+ int result = err->code;
+ g_error_free(err);
+ return result;
+ }

return session_request(session, transfer, func, user_data);
}
@@ -976,7 +989,7 @@ int obc_session_send(struct obc_session *session, const char *filename,
{
struct obc_transfer *transfer;
const char *agent;
- int err;
+ GError *err = NULL;

if (session->obex == NULL)
return -ENOTCONN;
@@ -986,14 +999,11 @@ int obc_session_send(struct obc_session *session, const char *filename,
transfer = obc_transfer_create(session->conn, agent,
OBC_TRANSFER_PUT,
filename,
- name, NULL, NULL);
- if (transfer == NULL)
- return -EINVAL;
-
- err = obc_transfer_set_file(transfer);
- if (err < 0) {
- obc_transfer_unregister(transfer);
- return err;
+ name, NULL, NULL, &err);
+ if (transfer == NULL) {
+ int result = err->code;
+ g_error_free(err);
+ return result;
}

return session_request(session, transfer, NULL, NULL);
@@ -1041,6 +1051,7 @@ int obc_session_put(struct obc_session *session, char *buf, const char *name)
{
struct obc_transfer *transfer;
const char *agent;
+ GError *err = NULL;

if (session->obex == NULL) {
g_free(buf);
@@ -1053,10 +1064,11 @@ int obc_session_put(struct obc_session *session, char *buf, const char *name)
OBC_TRANSFER_PUT,
buf, strlen(buf),
g_free, name, NULL,
- NULL);
+ NULL, &err);
if (transfer == NULL) {
- g_free(buf);
- return -EIO;
+ int result = err->code;
+ g_error_free(err);
+ return result;
}

return session_request(session, transfer, NULL, NULL);
diff --git a/client/transfer.c b/client/transfer.c
index a177eb3..9e6aab2 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -28,6 +28,7 @@
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
+#include <stdlib.h>
#include <assert.h>
#include <string.h>
#include <sys/stat.h>
@@ -311,7 +312,8 @@ static struct obc_transfer *transfer_create(DBusConnection *conn,
const char *type,
struct obc_transfer_params *params,
struct file_location *file_location,
- struct mem_location *mem_location)
+ struct mem_location *mem_location,
+ GError **err)
{
struct obc_transfer *transfer;

@@ -340,6 +342,8 @@ static struct obc_transfer *transfer_create(DBusConnection *conn,
transfer->conn = dbus_bus_get(DBUS_BUS_SESSION, NULL);
if (transfer->conn == NULL) {
obc_transfer_free(transfer);
+ g_set_error(err, OBC_TRANSFER_ERROR, -EIO,
+ "Unable to register transfer");
return NULL;
}

@@ -348,6 +352,8 @@ static struct obc_transfer *transfer_create(DBusConnection *conn,
obc_transfer_methods, NULL, NULL,
transfer, NULL) == FALSE) {
obc_transfer_free(transfer);
+ g_set_error(err, OBC_TRANSFER_ERROR, -EIO,
+ "Unable to register transfer");
return NULL;
}

@@ -369,23 +375,71 @@ void obc_transfer_unregister(struct obc_transfer *transfer)
obc_transfer_free(transfer);
}

+static gboolean transfer_open_file(struct obc_transfer *transfer, GError **err)
+{
+ struct file_location *location = transfer->file_location;
+ int fd;
+ struct stat st;
+
+ if (transfer->command == OBC_TRANSFER_PUT) {
+ DBG("opening file: %s", location->filename);
+ fd = open(location->filename, O_RDONLY);
+ } else {
+ fd = open(location->filename ? : transfer->name,
+ O_WRONLY | O_CREAT | O_TRUNC, 0600);
+ DBG("creating file: %s", location->filename);
+ }
+
+ if (fd < 0) {
+ error("open(): %s (%d)", strerror(errno), errno);
+ g_set_error(err, OBC_TRANSFER_ERROR, -EIO,
+ "Cannot open file");
+ return FALSE;
+ }
+
+ if (transfer->command == OBC_TRANSFER_PUT) {
+ if (fstat(fd, &st) < 0) {
+ error("fstat(): %s (%d)", strerror(errno), errno);
+ g_set_error(err, OBC_TRANSFER_ERROR, -EIO,
+ "Cannot get file size");
+ return FALSE;
+ }
+
+ transfer->size = st.st_size;
+ }
+
+ location->fd = fd;
+ return TRUE;
+}
+
struct obc_transfer *obc_transfer_create(DBusConnection *conn,
const char *agent,
ObcTransferCommand cmd,
const char *filename,
const char *name,
const char *type,
- struct obc_transfer_params *params)
+ struct obc_transfer_params *params,
+ GError **err)
{
struct file_location *file_location;
+ struct obc_transfer *transfer;

assert(filename != NULL);

file_location = g_malloc0(sizeof(*file_location));
file_location->filename = g_strdup(filename);

- return transfer_create(conn, agent, cmd, name, type, params,
- file_location, NULL);
+ transfer = transfer_create(conn, agent, cmd, name, type, params,
+ file_location, NULL, err);
+ if (transfer == NULL)
+ return NULL;
+
+ if (!transfer_open_file(transfer, err)) {
+ obc_transfer_free(transfer);
+ return NULL;
+ }
+
+ return transfer;
}

struct obc_transfer *obc_transfer_create_mem(DBusConnection *conn,
@@ -395,7 +449,8 @@ struct obc_transfer *obc_transfer_create_mem(DBusConnection *conn,
GDestroyNotify buffer_destroy_func,
const char *name,
const char *type,
- struct obc_transfer_params *params)
+ struct obc_transfer_params *params,
+ GError **err)
{
struct mem_location *mem_location;
struct obc_transfer *transfer;
@@ -419,7 +474,7 @@ struct obc_transfer *obc_transfer_create_mem(DBusConnection *conn,
}

transfer = transfer_create(conn, agent, cmd, name, type, params,
- NULL, mem_location);
+ NULL, mem_location, err);
if (transfer == NULL)
return NULL;

@@ -654,18 +709,6 @@ static gboolean transfer_start_get(struct obc_transfer *transfer, GError **err)
if (transfer->mem_location != NULL)
rsp_cb = get_buf_xfer_progress;
else {
- const char *filename = transfer->file_location->filename;
- int fd = open(filename ? : transfer->name,
- O_WRONLY | O_CREAT, 0600);
-
- if (fd < 0) {
- error("open(): %s(%d)", strerror(errno), errno);
- g_set_error(err, OBC_TRANSFER_ERROR, -EIO,
- "Cannot open file");
- return FALSE;
- }
-
- transfer->file_location->fd = fd;
data_cb = get_xfer_progress;
complete_cb = xfer_complete;
}
@@ -791,11 +834,35 @@ void obc_transfer_set_name(struct obc_transfer *transfer, const char *name)
transfer->name = g_strdup(name);
}

-void obc_transfer_set_filename(struct obc_transfer *transfer,
- const char *filename)
+gboolean obc_transfer_set_filename(struct obc_transfer *transfer,
+ const char *filename,
+ GError **err)
{
- g_free(transfer->file_location->filename);
- transfer->file_location->filename = g_strdup(filename);
+ struct file_location *location = transfer->file_location;
+ char *old = location->filename;
+
+ location->filename = g_strdup(filename);
+
+ if ((old == NULL) || (filename == NULL) || !g_strcmp0(old, filename))
+ goto done;
+
+ if (location->fd > 0) {
+ close(location->fd);
+ location->fd = 0;
+
+ if ((transfer->command == OBC_TRANSFER_GET) &&
+ (transfer->file_location != NULL))
+ unlink(transfer->file_location->filename);
+ }
+
+ if (!transfer_open_file(transfer, err)) {
+ g_free(old);
+ return FALSE;
+ }
+
+done:
+ g_free(old);
+ return TRUE;
}

const char *obc_transfer_get_path(struct obc_transfer *transfer)
@@ -807,29 +874,3 @@ gint64 obc_transfer_get_size(struct obc_transfer *transfer)
{
return transfer->size;
}
-
-int obc_transfer_set_file(struct obc_transfer *transfer)
-{
- int fd;
- struct stat st;
-
- if (transfer->file_location != NULL)
- return 0;
-
- fd = open(transfer->file_location->filename, O_RDONLY);
- if (fd < 0) {
- error("open(): %s(%d)", strerror(errno), errno);
- return -errno;
- }
-
- if (fstat(fd, &st) < 0) {
- error("fstat(): %s(%d)", strerror(errno), errno);
- close(fd);
- return -errno;
- }
-
- transfer->file_location->fd = fd;
- transfer->size = st.st_size;
-
- return 0;
-}
diff --git a/client/transfer.h b/client/transfer.h
index 09c708d..166d08c 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -44,7 +44,8 @@ struct obc_transfer *obc_transfer_create(DBusConnection *conn,
const char *filename,
const char *name,
const char *type,
- struct obc_transfer_params *params);
+ struct obc_transfer_params *params,
+ GError **err);

/* similar as above, but from memory. for get operations, buffer must be NULL */
struct obc_transfer *obc_transfer_create_mem(DBusConnection *conn,
@@ -54,7 +55,8 @@ struct obc_transfer *obc_transfer_create_mem(DBusConnection *conn,
GDestroyNotify buffer_destroy_func,
const char *name,
const char *type,
- struct obc_transfer_params *params);
+ struct obc_transfer_params *params,
+ GError **err);

void obc_transfer_unregister(struct obc_transfer *transfer);

@@ -72,8 +74,8 @@ const void *obc_transfer_get_params(struct obc_transfer *transfer,
const void *obc_transfer_get_buffer(struct obc_transfer *transfer,
size_t *size);
void obc_transfer_set_name(struct obc_transfer *transfer, const char *name);
-void obc_transfer_set_filename(struct obc_transfer *transfer,
- const char *filename);
+gboolean obc_transfer_set_filename(struct obc_transfer *transfer,
+ const char *filename,
+ GError **err);
const char *obc_transfer_get_path(struct obc_transfer *transfer);
gint64 obc_transfer_get_size(struct obc_transfer *transfer);
-int obc_transfer_set_file(struct obc_transfer *transfer);
--
1.7.7.6


2012-04-10 13:38:31

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 10/11] client: unify transfer implementation

From: Mikel Astiz <[email protected]>

This patch simplifies the internal handling of transfers by unifying put
and get operations, while also sharing most of the code between memory-
based and file-based transfers.
---
client/transfer.c | 333 +++++++++++++++++++++-------------------------------
1 files changed, 135 insertions(+), 198 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index 04dd178..c7d74a9 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -183,7 +183,7 @@ static void transfer_notify_error(struct obc_transfer *transfer, GError *err)
struct transfer_callback *callback = transfer->callback;
struct file_location *location = transfer->file_location;

- if ((transfer->direction == OBC_TRANSFER_GET) && (location != NULL)) {
+ if ((transfer->command == OBC_TRANSFER_GET) && (location != NULL)) {
if (location->fd > 0) {
close(location->fd);
location->fd = 0;
@@ -494,86 +494,14 @@ struct obc_transfer *obc_transfer_create_mem(DBusConnection *conn,
return transfer;
}

-static void obc_transfer_read(struct obc_transfer *transfer,
- const void *buf, gsize len)
+static void handle_get_apparams(struct obc_transfer *transfer, GObexPacket *rsp)
{
- struct mem_location *location = transfer->mem_location;
- gsize bsize;
-
- assert(location != NULL);
-
- /* copy all buffered data */
- bsize = location->buffer_len - transfer->transferred;
-
- if (bsize < len) {
- location->buffer_len += len - bsize;
- location->buffer = g_realloc(location->buffer,
- location->buffer_len);
- }
-
- memcpy(location->buffer + transfer->transferred, buf, len);
-
- transfer->transferred += len;
-}
-
-static void get_buf_xfer_complete(GObex *obex, GError *err, gpointer user_data)
-{
- struct obc_transfer *transfer = user_data;
- struct mem_location *location = transfer->mem_location;
- gsize bsize;
-
- assert(location != NULL);
-
- transfer->xfer = 0;
-
- if (err)
- goto done;
-
- if (transfer->transferred > 0 &&
- ((char *) location->buffer)[transfer->transferred - 1] == '\0')
- goto done;
-
- bsize = location->buffer_len - transfer->transferred;
- if (bsize < 1) {
- location->buffer_len += 1;
- location->buffer = g_realloc(location->buffer,
- location->buffer_len);
- }
-
- ((char *) location->buffer)[transfer->transferred] = '\0';
- transfer->size = strlen(location->buffer);
-
-done:
- transfer_notify_complete(transfer);
-}
-
-static void get_buf_xfer_progress(GObex *obex, GError *err, GObexPacket *rsp,
- gpointer user_data)
-{
- struct obc_transfer *transfer = user_data;
- GObexPacket *req;
GObexHeader *hdr;
const guint8 *buf;
gsize len;
- guint8 rspcode;
- gboolean final;
-
- if (err != NULL) {
- get_buf_xfer_complete(obex, err, transfer);
- return;
- }
-
- rspcode = g_obex_packet_get_operation(rsp, &final);
- if (rspcode != G_OBEX_RSP_SUCCESS && rspcode != G_OBEX_RSP_CONTINUE) {
- err = g_error_new(OBC_TRANSFER_ERROR, rspcode,
- "Transfer failed (0x%02x)", rspcode);
- get_buf_xfer_complete(obex, err, transfer);
- g_error_free(err);
- return;
- }

hdr = g_obex_packet_get_header(rsp, G_OBEX_HDR_APPARAM);
- if (hdr) {
+ if (hdr != NULL) {
g_obex_header_get_bytes(hdr, &buf, &len);
if (len != 0) {
if (transfer->params == NULL)
@@ -586,103 +514,104 @@ static void get_buf_xfer_progress(GObex *obex, GError *err, GObexPacket *rsp,
transfer->params->size = len;
}
}
-
- hdr = g_obex_packet_get_body(rsp);
- if (hdr) {
- g_obex_header_get_bytes(hdr, &buf, &len);
- if (len != 0)
- obc_transfer_read(transfer, buf, len);
- }
-
- if (rspcode == G_OBEX_RSP_SUCCESS) {
- get_buf_xfer_complete(obex, err, transfer);
- return;
- }
-
- if (!g_obex_srm_active(obex)) {
- req = g_obex_packet_new(G_OBEX_OP_GET, TRUE, G_OBEX_HDR_INVALID);
-
- transfer->xfer = g_obex_send_req(obex, req, -1,
- get_buf_xfer_progress,
- transfer, &err);
- }
-
- transfer_notify_progress(transfer);
}

-static void xfer_complete(GObex *obex, GError *err, gpointer user_data)
+static gboolean handle_get_body(struct obc_transfer *transfer, GObexPacket *rsp)
{
- struct obc_transfer *transfer = user_data;
+ GObexHeader *body = g_obex_packet_get_body(rsp);
+ GError *err;
+ const guint8 *buf;
+ gsize len;

- transfer->xfer = 0;
+ if (body == NULL)
+ return TRUE;

- if (err != NULL) {
- transfer_notify_error(transfer, err);
- return;
- }
+ g_obex_header_get_bytes(body, &buf, &len);
+ if (len == 0)
+ return TRUE;

- transfer->size = transfer->transferred;
+ if (transfer->file_location != NULL) {
+ struct file_location *location = transfer->file_location;

- transfer_notify_complete(transfer);
-}
+ if (write(location->fd, buf, len) < (ssize_t) len) {
+ err = g_error_new(OBC_TRANSFER_ERROR, -EIO,
+ "Write failed");
+ goto failed;
+ }
+ } else {
+ struct mem_location *location = transfer->mem_location;
+ gsize bsize;

-static gboolean get_xfer_progress(const void *buf, gsize len,
- gpointer user_data)
-{
- struct obc_transfer *transfer = user_data;
- struct file_location *location = transfer->file_location;
+ assert(location != NULL);

- assert(location != NULL);
- assert(location->fd > 0);
+ /* copy all buffered data */
+ bsize = location->buffer_len - transfer->transferred;

- if (write(location->fd, buf, len) < (ssize_t) len)
- return FALSE;
+ /* for convenience, leave space for final null character */
+ if (bsize < len + 1) {
+ location->buffer_len += len + 1 - bsize;
+ location->buffer = g_realloc(location->buffer,
+ location->buffer_len);
+ }
+
+ memcpy(location->buffer + transfer->transferred, buf, len);
+ }
+
+ transfer->transferred += len;

transfer_notify_progress(transfer);

return TRUE;
+
+failed:
+ error("%s", err->message);
+ transfer_notify_error(transfer, err);
+ g_clear_error(&err);
+ return FALSE;
}

-static gssize put_buf_xfer_progress(void *buf, gsize len, gpointer user_data)
+static gssize put_get_data(void *buf, gsize len, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
- struct mem_location *location = transfer->mem_location;
- gsize size;
+ GObexPacket *req;
+ GError *err = NULL;
+ gssize size;

- assert(location != NULL);
+ if (transfer->file_location != NULL) {
+ size = read(transfer->file_location->fd, buf, len);
+ if (size < 0)
+ goto failed;
+ } else {
+ struct mem_location *location = transfer->mem_location;

- if (transfer->transferred == transfer->size)
- return 0;
+ assert(location != NULL);

- size = transfer->size - transfer->transferred;
- size = len > size ? len : size;
- if (size == 0)
- return 0;
+ if (transfer->transferred == transfer->size)
+ return 0;

- memcpy(buf, location->buffer + transfer->transferred, size);
+ size = transfer->size - transfer->transferred;
+ size = (gssize) len > size ? (gssize) len : size;
+
+ if (size > 0)
+ memcpy(buf, location->buffer + transfer->transferred,
+ size);
+ }

transfer->transferred += size;

transfer_notify_progress(transfer);

return size;
-}
-
-static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
-{
- struct obc_transfer *transfer = user_data;
- struct file_location *location = transfer->file_location;
- gssize size;
-
- size = read(location->fd, buf, len);
- if (size <= 0)
- return size;

- transfer->transferred += size;
+failed:
+ err = g_error_new(OBC_TRANSFER_ERROR, -EIO, "Read failed");
+ transfer_notify_error(transfer, err);
+ g_clear_error(&err);

- transfer_notify_progress(transfer);
+ req = g_obex_packet_new(G_OBEX_OP_ABORT, TRUE, G_OBEX_HDR_INVALID);
+ g_obex_send_req(transfer->obex, req, -1, NULL, NULL, NULL);

- return size;
+ return -1;
}

gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
@@ -703,59 +632,67 @@ gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
return TRUE;
}

-static gboolean transfer_start_get(struct obc_transfer *transfer, GError **err)
+static void xfer_response(GObex *obex, GError *err, GObexPacket *rsp,
+ gpointer user_data)
{
+ struct obc_transfer *transfer = user_data;
GObexPacket *req;
- GObexDataConsumer data_cb;
- GObexFunc complete_cb;
- GObexResponseFunc rsp_cb = NULL;
+ gboolean rspcode, final;

- if (transfer->xfer > 0) {
- g_set_error(err, OBC_TRANSFER_ERROR, -EALREADY,
- "Transfer already started");
- return FALSE;
+ transfer->xfer = 0;
+
+ if (err != NULL) {
+ transfer_notify_error(transfer, err);
+ return;
}

- if (transfer->mem_location != NULL)
- rsp_cb = get_buf_xfer_progress;
- else {
- data_cb = get_xfer_progress;
- complete_cb = xfer_complete;
+ rspcode = g_obex_packet_get_operation(rsp, &final);
+ if (rspcode != G_OBEX_RSP_SUCCESS && rspcode != G_OBEX_RSP_CONTINUE) {
+ err = g_error_new(OBC_TRANSFER_ERROR, rspcode,
+ "Transfer failed (0x%02x)", rspcode);
+ transfer_notify_error(transfer, err);
+ g_error_free(err);
+ return;
}

- req = g_obex_packet_new(G_OBEX_OP_GET, TRUE, G_OBEX_HDR_INVALID);
+ if (transfer->command == OBC_TRANSFER_GET) {
+ handle_get_apparams(transfer, rsp);

- if (transfer->name != NULL)
- g_obex_packet_add_unicode(req, G_OBEX_HDR_NAME,
- transfer->name);
+ if (handle_get_body(transfer, rsp) == FALSE)
+ return;
+ }

- if (transfer->type != NULL)
- g_obex_packet_add_bytes(req, G_OBEX_HDR_TYPE, transfer->type,
- strlen(transfer->type) + 1);
+ if (rspcode == G_OBEX_RSP_SUCCESS) {
+ if (transfer->mem_location != NULL) {
+ char *buf = transfer->mem_location->buffer;

- if (transfer->params != NULL)
- g_obex_packet_add_bytes(req, G_OBEX_HDR_APPARAM,
- transfer->params->data,
- transfer->params->size);
+ buf[transfer->transferred] = '\0';
+ }

- if (rsp_cb)
- transfer->xfer = g_obex_send_req(transfer->obex, req, -1,
- rsp_cb, transfer, err);
- else
- transfer->xfer = g_obex_get_req_pkt(transfer->obex, req,
- data_cb, complete_cb, transfer,
- err);
+ transfer->size = transfer->transferred;

- if (transfer->xfer == 0)
- return FALSE;
+ transfer_notify_complete(transfer);
+ return;
+ }

- return TRUE;
+ if (transfer->command == OBC_TRANSFER_PUT) {
+ req = g_obex_packet_new(G_OBEX_OP_PUT, FALSE,
+ G_OBEX_HDR_INVALID);
+ g_obex_packet_add_body(req, put_get_data, transfer);
+ } else if (!g_obex_srm_active(transfer->obex)) {
+ req = g_obex_packet_new(G_OBEX_OP_GET, TRUE,
+ G_OBEX_HDR_INVALID);
+ } else
+ return;
+
+ transfer->xfer = g_obex_send_req(obex, req, -1, xfer_response,
+ transfer, &err);
}

-static gboolean transfer_start_put(struct obc_transfer *transfer, GError **err)
+static gboolean transfer_start_obex(struct obc_transfer *transfer, GError **err)
{
GObexPacket *req;
- GObexDataProducer data_cb;
+ guint8 opcode;

if (transfer->xfer > 0) {
g_set_error(err, OBC_TRANSFER_ERROR, -EALREADY,
@@ -763,12 +700,12 @@ static gboolean transfer_start_put(struct obc_transfer *transfer, GError **err)
return FALSE;
}

- if (transfer->mem_location != NULL)
- data_cb = put_buf_xfer_progress;
+ if (transfer->command == OBC_TRANSFER_PUT)
+ opcode = G_OBEX_OP_PUT;
else
- data_cb = put_xfer_progress;
+ opcode = G_OBEX_OP_GET;

- req = g_obex_packet_new(G_OBEX_OP_PUT, FALSE, G_OBEX_HDR_INVALID);
+ req = g_obex_packet_new(opcode, FALSE, G_OBEX_HDR_INVALID);

if (transfer->name != NULL)
g_obex_packet_add_unicode(req, G_OBEX_HDR_NAME,
@@ -778,37 +715,37 @@ static gboolean transfer_start_put(struct obc_transfer *transfer, GError **err)
g_obex_packet_add_bytes(req, G_OBEX_HDR_TYPE, transfer->type,
strlen(transfer->type) + 1);

- if (transfer->size < UINT32_MAX)
- g_obex_packet_add_uint32(req, G_OBEX_HDR_LENGTH, transfer->size);
+ if (transfer->command == OBC_TRANSFER_PUT) {
+ if (transfer->size < UINT32_MAX)
+ g_obex_packet_add_uint32(req, G_OBEX_HDR_LENGTH,
+ transfer->size);
+ }

if (transfer->params != NULL)
g_obex_packet_add_bytes(req, G_OBEX_HDR_APPARAM,
transfer->params->data,
transfer->params->size);

- transfer->xfer = g_obex_put_req_pkt(transfer->obex, req, data_cb,
- xfer_complete, transfer,
- err);
- if (transfer->xfer == 0)
- return FALSE;
+ transfer->xfer = g_obex_send_req(transfer->obex, req, -1, xfer_response,
+ transfer, err);
+ if (transfer->xfer > 0)
+ return TRUE;

- return TRUE;
+ return FALSE;
}

gboolean obc_transfer_start(struct obc_transfer *transfer, GObex *obex,
GError **err)
{
- transfer->obex = g_obex_ref(obex);
-
- switch (transfer->command) {
- case OBC_TRANSFER_GET:
- return transfer_start_get(transfer, err);
- case OBC_TRANSFER_PUT:
- return transfer_start_put(transfer, err);
+ if (transfer->xfer > 0) {
+ g_set_error(err, OBC_TRANSFER_ERROR, -EALREADY,
+ "Transfer already started");
+ return FALSE;
}

- g_set_error(err, OBC_TRANSFER_ERROR, -ENOTSUP, "Not supported");
- return FALSE;
+ transfer->obex = g_obex_ref(obex);
+
+ return transfer_start_obex(transfer, err);
}

ObcTransferCommand obc_transfer_get_cmd(struct obc_transfer *transfer)
--
1.7.7.6


2012-04-10 13:38:30

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 09/11] client: delete file in case of error

From: Mikel Astiz <[email protected]>

If a get operation fails, the file has not been written completely and
thus should be deleted.
---
client/transfer.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index 9e6aab2..04dd178 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -181,6 +181,16 @@ static void transfer_notify_complete(struct obc_transfer *transfer)
static void transfer_notify_error(struct obc_transfer *transfer, GError *err)
{
struct transfer_callback *callback = transfer->callback;
+ struct file_location *location = transfer->file_location;
+
+ if ((transfer->direction == OBC_TRANSFER_GET) && (location != NULL)) {
+ if (location->fd > 0) {
+ close(location->fd);
+ location->fd = 0;
+ }
+
+ unlink(location->filename);
+ }

if (callback == NULL)
return;
--
1.7.7.6


2012-04-10 13:38:26

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 05/11] client: transfer api merges put and get

From: Mikel Astiz <[email protected]>

A new enum type is used to distinguish put and get transfers.

This is more convenient since it is done when registering the transfer,
and not when it is actually started. The main benefits would be:
- Some actions can be taken during creation, such as opening files.
- session.c gets simplified.
- The size of a put transfer can be exposed in D-Bus, while queued.
- The transfer command (put or get) can be exposed in D-Bus.

None of these D-Bus changes are included in this patch.
---
client/session.c | 69 +++++++++++++++++------------------------------------
client/transfer.c | 59 +++++++++++++++++++++++++++++++++------------
client/transfer.h | 10 ++++++-
3 files changed, 73 insertions(+), 65 deletions(-)

diff --git a/client/session.c b/client/session.c
index c597bac..f27f45a 100644
--- a/client/session.c
+++ b/client/session.c
@@ -103,7 +103,7 @@ struct obc_session {

static GSList *sessions = NULL;

-static void session_prepare_put(gpointer data, gpointer user_data);
+static void session_start_transfer(gpointer data, gpointer user_data);
static void session_terminate_transfer(struct obc_session *session,
struct obc_transfer *transfer,
GError *gerr);
@@ -686,7 +686,7 @@ static void session_request_reply(DBusPendingCall *call, gpointer user_data)
DBG("Agent.Request() reply: %s", name);

if (strlen(name)) {
- if (p->auth_complete == session_prepare_put)
+ if (obc_transfer_get_cmd(transfer) == OBC_TRANSFER_PUT)
obc_transfer_set_name(transfer, name);
else
obc_transfer_set_filename(transfer, name);
@@ -731,7 +731,6 @@ static int pending_request_auth(struct pending_request *p)

static int session_request(struct obc_session *session,
struct obc_transfer *transfer,
- GFunc auth_complete,
session_callback_t func,
void *data)
{
@@ -740,7 +739,8 @@ static int session_request(struct obc_session *session,

obc_transfer_set_callback(transfer, transfer_progress, session);

- p = pending_request_new(session, transfer, auth_complete, func, data);
+ p = pending_request_new(session, transfer, session_start_transfer,
+ func, data);

if (session->p) {
g_queue_push_tail(session->queue, p);
@@ -915,19 +915,15 @@ fail:
session_notify_error(session, transfer, err);
}

-static void session_prepare_get(gpointer data, gpointer user_data)
+static void session_start_transfer(gpointer data, gpointer user_data)
{
struct obc_session *session = data;
struct obc_transfer *transfer = user_data;
- int ret;
+ GError *err = NULL;

- ret = obc_transfer_get(transfer);
- if (ret < 0) {
- GError *gerr = NULL;
-
- g_set_error(&gerr, OBEX_IO_ERROR, ret, "%s", strerror(-ret));
- session_notify_error(session, transfer, gerr);
- g_clear_error(&gerr);
+ if (!obc_transfer_start(transfer, &err)) {
+ session_notify_error(session, transfer, err);
+ g_clear_error(&err);
return;
}

@@ -959,8 +955,10 @@ int obc_session_get(struct obc_session *session, const char *type,
agent = NULL;

transfer = obc_transfer_register(session->conn, session->obex,
- agent, targetfile,
- name, type, params);
+ agent,
+ OBC_TRANSFER_GET,
+ targetfile, name, type,
+ params);
if (transfer == NULL) {
if (params != NULL) {
g_free(params->data);
@@ -969,8 +967,7 @@ int obc_session_get(struct obc_session *session, const char *type,
return -EIO;
}

- return session_request(session, transfer, session_prepare_get,
- func, user_data);
+ return session_request(session, transfer, func, user_data);
}

int obc_session_send(struct obc_session *session, const char *filename,
@@ -985,8 +982,9 @@ int obc_session_send(struct obc_session *session, const char *filename,

agent = obc_agent_get_name(session->agent);

- transfer = obc_transfer_register(session->conn, session->obex,
- agent, filename,
+ transfer = obc_transfer_register(session->conn, session->obex, agent,
+ OBC_TRANSFER_PUT,
+ filename,
name, NULL, NULL);
if (transfer == NULL)
return -EINVAL;
@@ -997,8 +995,7 @@ int obc_session_send(struct obc_session *session, const char *filename,
return err;
}

- return session_request(session, transfer, session_prepare_put,
- NULL, NULL);
+ return session_request(session, transfer, NULL, NULL);
}

int obc_session_pull(struct obc_session *session,
@@ -1039,26 +1036,6 @@ fail:
return NULL;
}

-static void session_prepare_put(gpointer data, gpointer user_data)
-{
- struct obc_session *session = data;
- struct obc_transfer *transfer = user_data;
- int ret;
-
- ret = obc_transfer_put(transfer);
- if (ret < 0) {
- GError *gerr = NULL;
-
- g_set_error(&gerr, OBEX_IO_ERROR, ret, "%s (%d)",
- strerror(-ret), -ret);
- session_notify_error(session, transfer, gerr);
- g_clear_error(&gerr);
- return;
- }
-
- DBG("Transfer(%p) started", transfer);
-}
-
int obc_session_put(struct obc_session *session, char *buf, const char *name)
{
struct obc_transfer *transfer;
@@ -1071,10 +1048,9 @@ int obc_session_put(struct obc_session *session, char *buf, const char *name)

agent = obc_agent_get_name(session->agent);

- transfer = obc_transfer_register(session->conn, session->obex,
- agent, NULL,
- name, NULL,
- NULL);
+ transfer = obc_transfer_register(session->conn, session->obex, agent,
+ OBC_TRANSFER_PUT,
+ name, NULL, NULL, NULL);
if (transfer == NULL) {
g_free(buf);
return -EIO;
@@ -1082,8 +1058,7 @@ int obc_session_put(struct obc_session *session, char *buf, const char *name)

obc_transfer_set_buffer(transfer, buf);

- return session_request(session, transfer, session_prepare_put,
- NULL, NULL);
+ return session_request(session, transfer, NULL, NULL);
}

static void agent_destroy(gpointer data, gpointer user_data)
diff --git a/client/transfer.c b/client/transfer.c
index b558d84..7f8c784 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -55,6 +55,7 @@ struct transfer_callback {

struct obc_transfer {
GObex *obex;
+ ObcTransferCommand command;
struct obc_transfer_params *params;
struct transfer_callback *callback;
DBusConnection *conn;
@@ -258,6 +259,7 @@ static void obc_transfer_free(struct obc_transfer *transfer)
struct obc_transfer *obc_transfer_register(DBusConnection *conn,
GObex *obex,
const char *agent,
+ ObcTransferCommand cmd,
const char *filename,
const char *name,
const char *type,
@@ -267,6 +269,7 @@ struct obc_transfer *obc_transfer_register(DBusConnection *conn,

transfer = g_new0(struct obc_transfer, 1);
transfer->obex = g_obex_ref(obex);
+ transfer->command = cmd;
transfer->agent = g_strdup(agent);
transfer->filename = g_strdup(filename);
transfer->name = g_strdup(name);
@@ -519,16 +522,18 @@ gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
return TRUE;
}

-int obc_transfer_get(struct obc_transfer *transfer)
+static gboolean transfer_start_get(struct obc_transfer *transfer, GError **err)
{
- GError *err = NULL;
GObexPacket *req;
GObexDataConsumer data_cb;
GObexFunc complete_cb;
GObexResponseFunc rsp_cb = NULL;

- if (transfer->xfer != 0)
- return -EALREADY;
+ if (transfer->xfer > 0) {
+ g_set_error(err, OBC_TRANSFER_ERROR, -EALREADY,
+ "Transfer already started");
+ return FALSE;
+ }

if (transfer->type != NULL &&
(strncmp(transfer->type, "x-obex/", 7) == 0 ||
@@ -539,7 +544,9 @@ int obc_transfer_get(struct obc_transfer *transfer)
O_WRONLY | O_CREAT, 0600);
if (fd < 0) {
error("open(): %s(%d)", strerror(errno), errno);
- return -errno;
+ g_set_error(err, OBC_TRANSFER_ERROR, -EIO,
+ "Cannot open file");
+ return FALSE;
}
transfer->fd = fd;
data_cb = get_xfer_progress;
@@ -563,26 +570,28 @@ int obc_transfer_get(struct obc_transfer *transfer)

if (rsp_cb)
transfer->xfer = g_obex_send_req(transfer->obex, req, -1,
- rsp_cb, transfer, &err);
+ rsp_cb, transfer, err);
else
transfer->xfer = g_obex_get_req_pkt(transfer->obex, req,
data_cb, complete_cb, transfer,
- &err);
+ err);

if (transfer->xfer == 0)
- return -ENOTCONN;
+ return FALSE;

- return 0;
+ return TRUE;
}

-int obc_transfer_put(struct obc_transfer *transfer)
+static gboolean transfer_start_put(struct obc_transfer *transfer, GError **err)
{
- GError *err = NULL;
GObexPacket *req;
GObexDataProducer data_cb;

- if (transfer->xfer != 0)
- return -EALREADY;
+ if (transfer->xfer > 0) {
+ g_set_error(err, OBC_TRANSFER_ERROR, -EALREADY,
+ "Transfer already started");
+ return FALSE;
+ }

if (transfer->buffer) {
data_cb = put_buf_xfer_progress;
@@ -612,11 +621,29 @@ done:

transfer->xfer = g_obex_put_req_pkt(transfer->obex, req, data_cb,
xfer_complete, transfer,
- &err);
+ err);
if (transfer->xfer == 0)
- return -ENOTCONN;
+ return FALSE;

- return 0;
+ return TRUE;
+}
+
+gboolean obc_transfer_start(struct obc_transfer *transfer, GError **err)
+{
+ switch (transfer->command) {
+ case OBC_TRANSFER_GET:
+ return transfer_start_get(transfer, err);
+ case OBC_TRANSFER_PUT:
+ return transfer_start_put(transfer, err);
+ }
+
+ g_set_error(err, OBC_TRANSFER_ERROR, -ENOTSUP, "Not supported");
+ return FALSE;
+}
+
+ObcTransferCommand obc_transfer_get_cmd(struct obc_transfer *transfer)
+{
+ return transfer->command;
}

const void *obc_transfer_get_params(struct obc_transfer *transfer, size_t *size)
diff --git a/client/transfer.h b/client/transfer.h
index c399129..fc86cce 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -21,6 +21,11 @@
*
*/

+typedef enum {
+ OBC_TRANSFER_GET,
+ OBC_TRANSFER_PUT
+} ObcTransferCommand;
+
struct obc_transfer_params {
void *data;
size_t size;
@@ -35,6 +40,7 @@ typedef void (*transfer_callback_t) (struct obc_transfer *transfer,
struct obc_transfer *obc_transfer_register(DBusConnection *conn,
GObex *obex,
const char *agent,
+ ObcTransferCommand cmd,
const char *filename,
const char *name,
const char *type,
@@ -46,9 +52,9 @@ gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
transfer_callback_t func,
void *user_data);

-int obc_transfer_get(struct obc_transfer *transfer);
-int obc_transfer_put(struct obc_transfer *transfer);
+gboolean obc_transfer_start(struct obc_transfer *transfer, GError **err);

+ObcTransferCommand obc_transfer_get_cmd(struct obc_transfer *transfer);
const void *obc_transfer_get_params(struct obc_transfer *transfer,
size_t *size);
const void *obc_transfer_get_buffer(struct obc_transfer *transfer,
--
1.7.7.6


2012-04-10 13:38:28

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 07/11] client: separate memory transfers in transfer api

From: Mikel Astiz <[email protected]>

The new transfer api clearly separates memory-based transfers from the
filesystem-based ones. This also simplifies buffer ownership semantics.

>From the implementation point of view, the data fields needed for the
two transfer types have been wrapped in separated data structures.
---
client/session.c | 26 +++---
client/transfer.c | 254 ++++++++++++++++++++++++++++++++++++++--------------
client/transfer.h | 15 +++-
3 files changed, 211 insertions(+), 84 deletions(-)

diff --git a/client/session.c b/client/session.c
index c628c6f..679627b 100644
--- a/client/session.c
+++ b/client/session.c
@@ -954,17 +954,19 @@ int obc_session_get(struct obc_session *session, const char *type,
else
agent = NULL;

- transfer = obc_transfer_register(session->conn, agent,
+ if (targetfile != NULL)
+ transfer = obc_transfer_create(session->conn, agent,
OBC_TRANSFER_GET,
targetfile, name, type,
params);
- if (transfer == NULL) {
- if (params != NULL) {
- g_free(params->data);
- g_free(params);
- }
+ else
+ transfer = obc_transfer_create_mem(session->conn, agent,
+ OBC_TRANSFER_GET,
+ NULL, 0, NULL,
+ name, type, params);
+
+ if (transfer == NULL)
return -EIO;
- }

return session_request(session, transfer, func, user_data);
}
@@ -981,7 +983,7 @@ int obc_session_send(struct obc_session *session, const char *filename,

agent = obc_agent_get_name(session->agent);

- transfer = obc_transfer_register(session->conn, agent,
+ transfer = obc_transfer_create(session->conn, agent,
OBC_TRANSFER_PUT,
filename,
name, NULL, NULL);
@@ -1047,16 +1049,16 @@ int obc_session_put(struct obc_session *session, char *buf, const char *name)

agent = obc_agent_get_name(session->agent);

- transfer = obc_transfer_register(session->conn, agent,
+ transfer = obc_transfer_create_mem(session->conn, agent,
OBC_TRANSFER_PUT,
- name, NULL, NULL, NULL);
+ buf, strlen(buf),
+ g_free, name, NULL,
+ NULL);
if (transfer == NULL) {
g_free(buf);
return -EIO;
}

- obc_transfer_set_buffer(transfer, buf);
-
return session_request(session, transfer, NULL, NULL);
}

diff --git a/client/transfer.c b/client/transfer.c
index ea011a4..a177eb3 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -28,6 +28,7 @@
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>
+#include <assert.h>
#include <string.h>
#include <sys/stat.h>
#include <inttypes.h>
@@ -53,22 +54,30 @@ struct transfer_callback {
void *data;
};

+struct file_location {
+ gchar *filename; /* Local filename */
+ int fd;
+};
+
+struct mem_location {
+ void *buffer; /* For Get, allocated internally */
+ gint64 buffer_len;
+ GDestroyNotify buffer_destroy_func;
+};
+
struct obc_transfer {
GObex *obex;
ObcTransferCommand command;
+ struct file_location *file_location;
+ struct mem_location *mem_location;
struct obc_transfer_params *params;
struct transfer_callback *callback;
DBusConnection *conn;
char *agent; /* Transfer agent */
char *path; /* Transfer path */
- gchar *filename; /* Transfer file location */
char *name; /* Transfer object name */
char *type; /* Transfer object type */
- int fd;
guint xfer;
- char *buffer;
- size_t buffer_len;
- int filled;
gint64 size;
gint64 transferred;
};
@@ -132,7 +141,10 @@ static DBusMessage *obc_transfer_get_properties(DBusConnection *connection,

append_entry(&dict, "Name", DBUS_TYPE_STRING, &transfer->name);
append_entry(&dict, "Size", DBUS_TYPE_UINT64, &transfer->size);
- append_entry(&dict, "Filename", DBUS_TYPE_STRING, &transfer->filename);
+
+ if (transfer->file_location != NULL)
+ append_entry(&dict, "Filename", DBUS_TYPE_STRING,
+ &transfer->file_location->filename);

dbus_message_iter_close_container(&iter, &dict);

@@ -225,6 +237,44 @@ static GDBusMethodTable obc_transfer_methods[] = {
{ }
};

+static void free_file_location(struct obc_transfer *transfer)
+{
+ struct file_location *location = transfer->file_location;
+
+ if (location == NULL)
+ return;
+
+ if (location->fd > 0)
+ close(location->fd);
+
+ g_free(location->filename);
+ g_free(location);
+
+ transfer->file_location = NULL;
+}
+
+static void free_mem_location(struct obc_transfer *transfer)
+{
+ struct mem_location *location = transfer->mem_location;
+
+ if (location == NULL)
+ return;
+
+ transfer->mem_location = NULL;
+
+ switch (transfer->command) {
+ case OBC_TRANSFER_GET:
+ g_free(location->buffer);
+ break;
+ case OBC_TRANSFER_PUT:
+ if (location->buffer_destroy_func != NULL)
+ location->buffer_destroy_func(location->buffer);
+ break;
+ }
+
+ g_free(location);
+}
+
static void obc_transfer_free(struct obc_transfer *transfer)
{
DBG("%p", transfer);
@@ -232,9 +282,6 @@ static void obc_transfer_free(struct obc_transfer *transfer)
if (transfer->xfer)
g_obex_cancel_transfer(transfer->xfer);

- if (transfer->fd > 0)
- close(transfer->fd);
-
if (transfer->params != NULL) {
g_free(transfer->params->data);
g_free(transfer->params);
@@ -246,30 +293,37 @@ static void obc_transfer_free(struct obc_transfer *transfer)
if (transfer->obex)
g_obex_unref(transfer->obex);

+ free_file_location(transfer);
+ free_mem_location(transfer);
+
g_free(transfer->callback);
g_free(transfer->agent);
- g_free(transfer->filename);
g_free(transfer->name);
g_free(transfer->type);
g_free(transfer->path);
- g_free(transfer->buffer);
g_free(transfer);
}

-struct obc_transfer *obc_transfer_register(DBusConnection *conn,
- const char *agent,
- ObcTransferCommand cmd,
- const char *filename,
- const char *name,
- const char *type,
- struct obc_transfer_params *params)
+static struct obc_transfer *transfer_create(DBusConnection *conn,
+ const char *agent,
+ ObcTransferCommand cmd,
+ const char *name,
+ const char *type,
+ struct obc_transfer_params *params,
+ struct file_location *file_location,
+ struct mem_location *mem_location)
{
struct obc_transfer *transfer;

+ assert(conn != NULL);
+ assert((type != NULL) || (name != NULL));
+ assert((file_location == NULL) != (mem_location == NULL));
+
transfer = g_new0(struct obc_transfer, 1);
transfer->command = cmd;
transfer->agent = g_strdup(agent);
- transfer->filename = g_strdup(filename);
+ transfer->file_location = file_location;
+ transfer->mem_location = mem_location;
transfer->name = g_strdup(name);
transfer->type = g_strdup(type);
transfer->params = params;
@@ -315,49 +369,114 @@ void obc_transfer_unregister(struct obc_transfer *transfer)
obc_transfer_free(transfer);
}

+struct obc_transfer *obc_transfer_create(DBusConnection *conn,
+ const char *agent,
+ ObcTransferCommand cmd,
+ const char *filename,
+ const char *name,
+ const char *type,
+ struct obc_transfer_params *params)
+{
+ struct file_location *file_location;
+
+ assert(filename != NULL);
+
+ file_location = g_malloc0(sizeof(*file_location));
+ file_location->filename = g_strdup(filename);
+
+ return transfer_create(conn, agent, cmd, name, type, params,
+ file_location, NULL);
+}
+
+struct obc_transfer *obc_transfer_create_mem(DBusConnection *conn,
+ const char *agent,
+ ObcTransferCommand cmd,
+ void *buffer, gint64 size,
+ GDestroyNotify buffer_destroy_func,
+ const char *name,
+ const char *type,
+ struct obc_transfer_params *params)
+{
+ struct mem_location *mem_location;
+ struct obc_transfer *transfer;
+
+ assert((cmd == OBC_TRANSFER_PUT) || (buffer == NULL));
+
+ mem_location = g_malloc0(sizeof(*mem_location));
+
+ switch (cmd) {
+ case OBC_TRANSFER_GET:
+ assert(buffer == NULL);
+ mem_location->buffer_len = 1024;
+ mem_location->buffer = g_malloc0(mem_location->buffer_len);
+ break;
+ case OBC_TRANSFER_PUT:
+ assert(buffer != NULL);
+ mem_location->buffer = buffer;
+ mem_location->buffer_len = size;
+ mem_location->buffer_destroy_func = buffer_destroy_func;
+ break;
+ }
+
+ transfer = transfer_create(conn, agent, cmd, name, type, params,
+ NULL, mem_location);
+ if (transfer == NULL)
+ return NULL;
+
+ if (cmd == OBC_TRANSFER_PUT)
+ transfer->size = size;
+
+ return transfer;
+}
+
static void obc_transfer_read(struct obc_transfer *transfer,
const void *buf, gsize len)
{
+ struct mem_location *location = transfer->mem_location;
gsize bsize;

+ assert(location != NULL);
+
/* copy all buffered data */
- bsize = transfer->buffer_len - transfer->filled;
+ bsize = location->buffer_len - transfer->transferred;

if (bsize < len) {
- transfer->buffer_len += len - bsize;
- transfer->buffer = g_realloc(transfer->buffer,
- transfer->buffer_len);
+ location->buffer_len += len - bsize;
+ location->buffer = g_realloc(location->buffer,
+ location->buffer_len);
}

- memcpy(transfer->buffer + transfer->filled, buf, len);
+ memcpy(location->buffer + transfer->transferred, buf, len);

- transfer->filled += len;
transfer->transferred += len;
}

static void get_buf_xfer_complete(GObex *obex, GError *err, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
+ struct mem_location *location = transfer->mem_location;
gsize bsize;

+ assert(location != NULL);
+
transfer->xfer = 0;

if (err)
goto done;

- if (transfer->filled > 0 &&
- transfer->buffer[transfer->filled - 1] == '\0')
+ if (transfer->transferred > 0 &&
+ ((char *) location->buffer)[transfer->transferred - 1] == '\0')
goto done;

- bsize = transfer->buffer_len - transfer->filled;
+ bsize = location->buffer_len - transfer->transferred;
if (bsize < 1) {
- transfer->buffer_len += 1;
- transfer->buffer = g_realloc(transfer->buffer,
- transfer->buffer_len);
+ location->buffer_len += 1;
+ location->buffer = g_realloc(location->buffer,
+ location->buffer_len);
}

- transfer->buffer[transfer->filled] = '\0';
- transfer->size = strlen(transfer->buffer);
+ ((char *) location->buffer)[transfer->transferred] = '\0';
+ transfer->size = strlen(location->buffer);

done:
transfer_notify_complete(transfer);
@@ -446,18 +565,13 @@ static gboolean get_xfer_progress(const void *buf, gsize len,
gpointer user_data)
{
struct obc_transfer *transfer = user_data;
+ struct file_location *location = transfer->file_location;

- obc_transfer_read(transfer, buf, len);
+ assert(location != NULL);
+ assert(location->fd > 0);

- if (transfer->fd > 0) {
- gint w;
-
- w = write(transfer->fd, transfer->buffer, transfer->filled);
- if (w < 0)
- return FALSE;
-
- transfer->filled -= w;
- }
+ if (write(location->fd, buf, len) < (ssize_t) len)
+ return FALSE;

transfer_notify_progress(transfer);

@@ -467,8 +581,11 @@ static gboolean get_xfer_progress(const void *buf, gsize len,
static gssize put_buf_xfer_progress(void *buf, gsize len, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
+ struct mem_location *location = transfer->mem_location;
gsize size;

+ assert(location != NULL);
+
if (transfer->transferred == transfer->size)
return 0;

@@ -477,7 +594,7 @@ static gssize put_buf_xfer_progress(void *buf, gsize len, gpointer user_data)
if (size == 0)
return 0;

- memcpy(buf, transfer->buffer + transfer->transferred, size);
+ memcpy(buf, location->buffer + transfer->transferred, size);

transfer->transferred += size;

@@ -489,9 +606,10 @@ static gssize put_buf_xfer_progress(void *buf, gsize len, gpointer user_data)
static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
+ struct file_location *location = transfer->file_location;
gssize size;

- size = read(transfer->fd, buf, len);
+ size = read(location->fd, buf, len);
if (size <= 0)
return size;

@@ -533,20 +651,21 @@ static gboolean transfer_start_get(struct obc_transfer *transfer, GError **err)
return FALSE;
}

- if (transfer->type != NULL &&
- (strncmp(transfer->type, "x-obex/", 7) == 0 ||
- strncmp(transfer->type, "x-bt/", 5) == 0)) {
+ if (transfer->mem_location != NULL)
rsp_cb = get_buf_xfer_progress;
- } else {
- int fd = open(transfer->filename ? : transfer->name,
+ else {
+ const char *filename = transfer->file_location->filename;
+ int fd = open(filename ? : transfer->name,
O_WRONLY | O_CREAT, 0600);
+
if (fd < 0) {
error("open(): %s(%d)", strerror(errno), errno);
g_set_error(err, OBC_TRANSFER_ERROR, -EIO,
"Cannot open file");
return FALSE;
}
- transfer->fd = fd;
+
+ transfer->file_location->fd = fd;
data_cb = get_xfer_progress;
complete_cb = xfer_complete;
}
@@ -591,14 +710,11 @@ static gboolean transfer_start_put(struct obc_transfer *transfer, GError **err)
return FALSE;
}

- if (transfer->buffer) {
+ if (transfer->mem_location != NULL)
data_cb = put_buf_xfer_progress;
- goto done;
- }
-
- data_cb = put_xfer_progress;
+ else
+ data_cb = put_xfer_progress;

-done:
req = g_obex_packet_new(G_OBEX_OP_PUT, FALSE, G_OBEX_HDR_INVALID);

if (transfer->name != NULL)
@@ -660,16 +776,13 @@ const void *obc_transfer_get_params(struct obc_transfer *transfer, size_t *size)

const void *obc_transfer_get_buffer(struct obc_transfer *transfer, size_t *size)
{
- if (size)
- *size = transfer->filled;
+ if (transfer->mem_location == NULL)
+ return NULL;

- return transfer->buffer;
-}
+ if (size != NULL)
+ *size = transfer->size;

-void obc_transfer_set_buffer(struct obc_transfer *transfer, char *buffer)
-{
- transfer->size = strlen(buffer);
- transfer->buffer = buffer;
+ return transfer->mem_location->buffer;
}

void obc_transfer_set_name(struct obc_transfer *transfer, const char *name)
@@ -681,8 +794,8 @@ void obc_transfer_set_name(struct obc_transfer *transfer, const char *name)
void obc_transfer_set_filename(struct obc_transfer *transfer,
const char *filename)
{
- g_free(transfer->filename);
- transfer->filename = g_strdup(filename);
+ g_free(transfer->file_location->filename);
+ transfer->file_location->filename = g_strdup(filename);
}

const char *obc_transfer_get_path(struct obc_transfer *transfer)
@@ -700,7 +813,10 @@ int obc_transfer_set_file(struct obc_transfer *transfer)
int fd;
struct stat st;

- fd = open(transfer->filename, O_RDONLY);
+ if (transfer->file_location != NULL)
+ return 0;
+
+ fd = open(transfer->file_location->filename, O_RDONLY);
if (fd < 0) {
error("open(): %s(%d)", strerror(errno), errno);
return -errno;
@@ -712,7 +828,7 @@ int obc_transfer_set_file(struct obc_transfer *transfer)
return -errno;
}

- transfer->fd = fd;
+ transfer->file_location->fd = fd;
transfer->size = st.st_size;

return 0;
diff --git a/client/transfer.h b/client/transfer.h
index a74ffea..09c708d 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -37,7 +37,8 @@ typedef void (*transfer_callback_t) (struct obc_transfer *transfer,
gint64 transferred, GError *err,
void *user_data);

-struct obc_transfer *obc_transfer_register(DBusConnection *conn,
+/* takes ownership of obc_transfer_params */
+struct obc_transfer *obc_transfer_create(DBusConnection *conn,
const char *agent,
ObcTransferCommand cmd,
const char *filename,
@@ -45,6 +46,16 @@ struct obc_transfer *obc_transfer_register(DBusConnection *conn,
const char *type,
struct obc_transfer_params *params);

+/* similar as above, but from memory. for get operations, buffer must be NULL */
+struct obc_transfer *obc_transfer_create_mem(DBusConnection *conn,
+ const char *agent,
+ ObcTransferCommand cmd,
+ void *buffer, gint64 size,
+ GDestroyNotify buffer_destroy_func,
+ const char *name,
+ const char *type,
+ struct obc_transfer_params *params);
+
void obc_transfer_unregister(struct obc_transfer *transfer);

gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
@@ -60,8 +71,6 @@ const void *obc_transfer_get_params(struct obc_transfer *transfer,
size_t *size);
const void *obc_transfer_get_buffer(struct obc_transfer *transfer,
size_t *size);
-void obc_transfer_set_buffer(struct obc_transfer *transfer, char *buffer);
-
void obc_transfer_set_name(struct obc_transfer *transfer, const char *name);
void obc_transfer_set_filename(struct obc_transfer *transfer,
const char *filename);
--
1.7.7.6


2012-04-10 13:38:27

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 06/11] client: transfers take gobex when starting

From: Mikel Astiz <[email protected]>

gobex api should not be used by a transfer until it is started. This
seems more explicit if the pointer is not passed during creation.
---
client/session.c | 9 ++++-----
client/transfer.c | 7 ++++---
client/transfer.h | 5 +++--
3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/client/session.c b/client/session.c
index f27f45a..c628c6f 100644
--- a/client/session.c
+++ b/client/session.c
@@ -921,7 +921,7 @@ static void session_start_transfer(gpointer data, gpointer user_data)
struct obc_transfer *transfer = user_data;
GError *err = NULL;

- if (!obc_transfer_start(transfer, &err)) {
+ if (!obc_transfer_start(transfer, session->obex, &err)) {
session_notify_error(session, transfer, err);
g_clear_error(&err);
return;
@@ -954,8 +954,7 @@ int obc_session_get(struct obc_session *session, const char *type,
else
agent = NULL;

- transfer = obc_transfer_register(session->conn, session->obex,
- agent,
+ transfer = obc_transfer_register(session->conn, agent,
OBC_TRANSFER_GET,
targetfile, name, type,
params);
@@ -982,7 +981,7 @@ int obc_session_send(struct obc_session *session, const char *filename,

agent = obc_agent_get_name(session->agent);

- transfer = obc_transfer_register(session->conn, session->obex, agent,
+ transfer = obc_transfer_register(session->conn, agent,
OBC_TRANSFER_PUT,
filename,
name, NULL, NULL);
@@ -1048,7 +1047,7 @@ int obc_session_put(struct obc_session *session, char *buf, const char *name)

agent = obc_agent_get_name(session->agent);

- transfer = obc_transfer_register(session->conn, session->obex, agent,
+ transfer = obc_transfer_register(session->conn, agent,
OBC_TRANSFER_PUT,
name, NULL, NULL, NULL);
if (transfer == NULL) {
diff --git a/client/transfer.c b/client/transfer.c
index 7f8c784..ea011a4 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -257,7 +257,6 @@ static void obc_transfer_free(struct obc_transfer *transfer)
}

struct obc_transfer *obc_transfer_register(DBusConnection *conn,
- GObex *obex,
const char *agent,
ObcTransferCommand cmd,
const char *filename,
@@ -268,7 +267,6 @@ struct obc_transfer *obc_transfer_register(DBusConnection *conn,
struct obc_transfer *transfer;

transfer = g_new0(struct obc_transfer, 1);
- transfer->obex = g_obex_ref(obex);
transfer->command = cmd;
transfer->agent = g_strdup(agent);
transfer->filename = g_strdup(filename);
@@ -628,8 +626,11 @@ done:
return TRUE;
}

-gboolean obc_transfer_start(struct obc_transfer *transfer, GError **err)
+gboolean obc_transfer_start(struct obc_transfer *transfer, GObex *obex,
+ GError **err)
{
+ transfer->obex = g_obex_ref(obex);
+
switch (transfer->command) {
case OBC_TRANSFER_GET:
return transfer_start_get(transfer, err);
diff --git a/client/transfer.h b/client/transfer.h
index fc86cce..a74ffea 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -38,7 +38,6 @@ typedef void (*transfer_callback_t) (struct obc_transfer *transfer,
void *user_data);

struct obc_transfer *obc_transfer_register(DBusConnection *conn,
- GObex *obex,
const char *agent,
ObcTransferCommand cmd,
const char *filename,
@@ -52,7 +51,9 @@ gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
transfer_callback_t func,
void *user_data);

-gboolean obc_transfer_start(struct obc_transfer *transfer, GError **err);
+gboolean obc_transfer_start(struct obc_transfer *transfer,
+ GObex *obex,
+ GError **err);

ObcTransferCommand obc_transfer_get_cmd(struct obc_transfer *transfer);
const void *obc_transfer_get_params(struct obc_transfer *transfer,
--
1.7.7.6


2012-04-10 13:38:25

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 04/11] client: fix progress report before field update

From: Mikel Astiz <[email protected]>

The progress report in put_xfer_progress was done before the
corresponding field was updated.
---
client/transfer.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index 3b4f8f2..b558d84 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -494,10 +494,10 @@ static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
if (size <= 0)
return size;

- transfer_notify_progress(transfer);
-
transfer->transferred += size;

+ transfer_notify_progress(transfer);
+
return size;
}

--
1.7.7.6


2012-04-10 13:38:24

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 03/11] client: refactor transfer callback use

From: Mikel Astiz <[email protected]>

Separated internal functions are used to report success, failure, and
progress, instead of using the callback directly.

This makes the code more readable.
---
client/transfer.c | 87 +++++++++++++++++++++++++++++++++--------------------
1 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index a609aa9..3b4f8f2 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -138,10 +138,48 @@ static DBusMessage *obc_transfer_get_properties(DBusConnection *connection,
return reply;
}

-static void obc_transfer_abort(struct obc_transfer *transfer)
+static void transfer_notify_progress(struct obc_transfer *transfer)
+{
+ struct transfer_callback *callback = transfer->callback;
+
+ DBG("%p", transfer);
+
+ if ((callback == NULL) || (transfer->transferred == transfer->size))
+ return;
+
+ callback->func(transfer, transfer->transferred, NULL, callback->data);
+}
+
+static void transfer_notify_complete(struct obc_transfer *transfer)
{
struct transfer_callback *callback = transfer->callback;

+ DBG("%p", transfer);
+
+ if (callback == NULL)
+ return;
+
+ transfer->callback = NULL;
+ callback->func(transfer, transfer->transferred, NULL, callback->data);
+ g_free(callback);
+}
+
+static void transfer_notify_error(struct obc_transfer *transfer, GError *err)
+{
+ struct transfer_callback *callback = transfer->callback;
+
+ if (callback == NULL)
+ return;
+
+ transfer->callback = NULL;
+ callback->func(transfer, transfer->transferred, err, callback->data);
+ g_free(callback);
+}
+
+static void obc_transfer_abort(struct obc_transfer *transfer)
+{
+ GError *err;
+
if (transfer->xfer > 0) {
g_obex_cancel_transfer(transfer->xfer);
transfer->xfer = 0;
@@ -152,15 +190,10 @@ static void obc_transfer_abort(struct obc_transfer *transfer)
transfer->obex = NULL;
}

- if (callback) {
- GError *err;
-
- err = g_error_new(OBC_TRANSFER_ERROR, -ECANCELED, "%s",
+ err = g_error_new(OBC_TRANSFER_ERROR, -ECANCELED, "%s",
strerror(ECANCELED));
- callback->func(transfer, transfer->transferred, err,
- callback->data);
- g_error_free(err);
- }
+ transfer_notify_error(transfer, err);
+ g_error_free(err);
}

static DBusMessage *obc_transfer_cancel(DBusConnection *connection,
@@ -304,7 +337,6 @@ static void obc_transfer_read(struct obc_transfer *transfer,
static void get_buf_xfer_complete(GObex *obex, GError *err, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
- struct transfer_callback *callback = transfer->callback;
gsize bsize;

transfer->xfer = 0;
@@ -327,15 +359,13 @@ static void get_buf_xfer_complete(GObex *obex, GError *err, gpointer user_data)
transfer->size = strlen(transfer->buffer);

done:
- if (callback)
- callback->func(transfer, transfer->size, err, callback->data);
+ transfer_notify_complete(transfer);
}

static void get_buf_xfer_progress(GObex *obex, GError *err, GObexPacket *rsp,
gpointer user_data)
{
struct obc_transfer *transfer = user_data;
- struct transfer_callback *callback = transfer->callback;
GObexPacket *req;
GObexHeader *hdr;
const guint8 *buf;
@@ -392,30 +422,29 @@ static void get_buf_xfer_progress(GObex *obex, GError *err, GObexPacket *rsp,
transfer, &err);
}

- if (callback && transfer->transferred != transfer->size)
- callback->func(transfer, transfer->transferred, err,
- callback->data);
+ transfer_notify_progress(transfer);
}

static void xfer_complete(GObex *obex, GError *err, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
- struct transfer_callback *callback = transfer->callback;

transfer->xfer = 0;

- if (err == NULL)
- transfer->size = transfer->transferred;
+ if (err != NULL) {
+ transfer_notify_error(transfer, err);
+ return;
+ }

- if (callback)
- callback->func(transfer, transfer->size, err, callback->data);
+ transfer->size = transfer->transferred;
+
+ transfer_notify_complete(transfer);
}

static gboolean get_xfer_progress(const void *buf, gsize len,
gpointer user_data)
{
struct obc_transfer *transfer = user_data;
- struct transfer_callback *callback = transfer->callback;

obc_transfer_read(transfer, buf, len);

@@ -429,9 +458,7 @@ static gboolean get_xfer_progress(const void *buf, gsize len,
transfer->filled -= w;
}

- if (callback && transfer->transferred != transfer->size)
- callback->func(transfer, transfer->transferred, NULL,
- callback->data);
+ transfer_notify_progress(transfer);

return TRUE;
}
@@ -439,7 +466,6 @@ static gboolean get_xfer_progress(const void *buf, gsize len,
static gssize put_buf_xfer_progress(void *buf, gsize len, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
- struct transfer_callback *callback = transfer->callback;
gsize size;

if (transfer->transferred == transfer->size)
@@ -454,9 +480,7 @@ static gssize put_buf_xfer_progress(void *buf, gsize len, gpointer user_data)

transfer->transferred += size;

- if (callback)
- callback->func(transfer, transfer->transferred, NULL,
- callback->data);
+ transfer_notify_progress(transfer);

return size;
}
@@ -464,16 +488,13 @@ static gssize put_buf_xfer_progress(void *buf, gsize len, gpointer user_data)
static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
{
struct obc_transfer *transfer = user_data;
- struct transfer_callback *callback = transfer->callback;
gssize size;

size = read(transfer->fd, buf, len);
if (size <= 0)
return size;

- if (callback)
- callback->func(transfer, transfer->transferred, NULL,
- callback->data);
+ transfer_notify_progress(transfer);

transfer->transferred += size;

--
1.7.7.6


2012-04-10 13:38:23

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 02/11] client: remove unused field

From: Mikel Astiz <[email protected]>

The errorcode field is set but never used, so it can safely be removed.
In addition there is no need for such a field, because errors can be
propagated using the available callback.
---
client/transfer.c | 22 +++++-----------------
1 files changed, 5 insertions(+), 17 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index 988ec10..a609aa9 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -70,7 +70,6 @@ struct obc_transfer {
int filled;
gint64 size;
gint64 transferred;
- int err;
};

static GQuark obc_transfer_error_quark(void)
@@ -310,10 +309,8 @@ static void get_buf_xfer_complete(GObex *obex, GError *err, gpointer user_data)

transfer->xfer = 0;

- if (err) {
- transfer->err = err->code;
+ if (err)
goto done;
- }

if (transfer->filled > 0 &&
transfer->buffer[transfer->filled - 1] == '\0')
@@ -407,14 +404,9 @@ static void xfer_complete(GObex *obex, GError *err, gpointer user_data)

transfer->xfer = 0;

- if (err) {
- transfer->err = err->code;
- goto done;
- }
-
- transfer->size = transfer->transferred;
+ if (err == NULL)
+ transfer->size = transfer->transferred;

-done:
if (callback)
callback->func(transfer, transfer->size, err, callback->data);
}
@@ -431,10 +423,8 @@ static gboolean get_xfer_progress(const void *buf, gsize len,
gint w;

w = write(transfer->fd, transfer->buffer, transfer->filled);
- if (w < 0) {
- transfer->err = -errno;
+ if (w < 0)
return FALSE;
- }

transfer->filled -= w;
}
@@ -478,10 +468,8 @@ static gssize put_xfer_progress(void *buf, gsize len, gpointer user_data)
gssize size;

size = read(transfer->fd, buf, len);
- if (size <= 0) {
- transfer->err = -errno;
+ if (size <= 0)
return size;
- }

if (callback)
callback->func(transfer, transfer->transferred, NULL,
--
1.7.7.6


2012-04-10 13:38:22

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v1 01/11] client: minor buffer access api changes

From: Mikel Astiz <[email protected]>

Trivial changes in buffer getters in both session and transfer:
- const qualifiers added, to avoid unwanted frees
- Buffers are now returned as void* instead of guint8*

Also, obc_session_get_buffer does not clear the internal buffer any
more. This doesn't have any impact in the current code though, since
transfers are unregistered immediately after.
---
client/pbap.c | 2 +-
client/session.c | 22 +++++-----------------
client/session.h | 4 ++--
client/transfer.c | 17 +++++++----------
client/transfer.h | 10 +++++-----
5 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/client/pbap.c b/client/pbap.c
index 53a608e..2e1cc10 100644
--- a/client/pbap.c
+++ b/client/pbap.c
@@ -294,7 +294,7 @@ static void pbap_setpath_cb(struct obc_session *session, GError *err,
static void read_return_apparam(struct obc_session *session,
guint16 *phone_book_size, guint8 *new_missed_calls)
{
- struct apparam_hdr *hdr;
+ const struct apparam_hdr *hdr;
size_t size;

*phone_book_size = 0;
diff --git a/client/session.c b/client/session.c
index a9883c7..c597bac 100644
--- a/client/session.c
+++ b/client/session.c
@@ -1156,10 +1156,9 @@ static struct obc_transfer *obc_session_get_transfer(
return session->p->transfer;
}

-const char *obc_session_get_buffer(struct obc_session *session, size_t *size)
+const void *obc_session_get_buffer(struct obc_session *session, size_t *size)
{
struct obc_transfer *transfer;
- const char *buf;

transfer = obc_session_get_transfer(session);
if (transfer == NULL) {
@@ -1169,29 +1168,18 @@ const char *obc_session_get_buffer(struct obc_session *session, size_t *size)
return NULL;
}

- buf = obc_transfer_get_buffer(transfer, size);
-
- obc_transfer_clear_buffer(transfer);
-
- return buf;
+ return obc_transfer_get_buffer(transfer, size);
}

-void *obc_session_get_params(struct obc_session *session, size_t *size)
+const void *obc_session_get_params(struct obc_session *session, size_t *size)
{
struct obc_transfer *transfer;
- struct obc_transfer_params params;

- transfer= obc_session_get_transfer(session);
+ transfer = obc_session_get_transfer(session);
if (transfer == NULL)
return NULL;

- if (obc_transfer_get_params(transfer, &params) < 0)
- return NULL;
-
- if (size)
- *size = params.size;
-
- return params.data;
+ return obc_transfer_get_params(transfer, size);
}

static void setpath_complete(struct obc_session *session, GError *err,
diff --git a/client/session.h b/client/session.h
index 4bfb41d..40be9ca 100644
--- a/client/session.h
+++ b/client/session.h
@@ -52,8 +52,8 @@ 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);
-const char *obc_session_get_buffer(struct obc_session *session, size_t *size);
-void *obc_session_get_params(struct obc_session *session, size_t *size);
+const void *obc_session_get_buffer(struct obc_session *session, size_t *size);
+const void *obc_session_get_params(struct obc_session *session, size_t *size);

int obc_session_send(struct obc_session *session, const char *filename,
const char *name);
diff --git a/client/transfer.c b/client/transfer.c
index 60c2e4f..988ec10 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -610,21 +610,18 @@ done:
return 0;
}

-int obc_transfer_get_params(struct obc_transfer *transfer,
- struct obc_transfer_params *params)
+const void *obc_transfer_get_params(struct obc_transfer *transfer, size_t *size)
{
- params->data = transfer->params->data;
- params->size = transfer->params->size;
+ if (transfer->params == NULL)
+ return NULL;

- return 0;
-}
+ if (size != NULL)
+ *size = transfer->params->size;

-void obc_transfer_clear_buffer(struct obc_transfer *transfer)
-{
- transfer->filled = 0;
+ return transfer->params->data;
}

-const char *obc_transfer_get_buffer(struct obc_transfer *transfer, size_t *size)
+const void *obc_transfer_get_buffer(struct obc_transfer *transfer, size_t *size)
{
if (size)
*size = transfer->filled;
diff --git a/client/transfer.h b/client/transfer.h
index da7d151..c399129 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -22,7 +22,7 @@
*/

struct obc_transfer_params {
- guint8 *data;
+ void *data;
size_t size;
};

@@ -49,11 +49,11 @@ gboolean obc_transfer_set_callback(struct obc_transfer *transfer,
int obc_transfer_get(struct obc_transfer *transfer);
int obc_transfer_put(struct obc_transfer *transfer);

-int obc_transfer_get_params(struct obc_transfer *transfer,
- struct obc_transfer_params *params);
-const char *obc_transfer_get_buffer(struct obc_transfer *transfer, size_t *size);
+const void *obc_transfer_get_params(struct obc_transfer *transfer,
+ size_t *size);
+const void *obc_transfer_get_buffer(struct obc_transfer *transfer,
+ size_t *size);
void obc_transfer_set_buffer(struct obc_transfer *transfer, char *buffer);
-void obc_transfer_clear_buffer(struct obc_transfer *transfer);

void obc_transfer_set_name(struct obc_transfer *transfer, const char *name);
void obc_transfer_set_filename(struct obc_transfer *transfer,
--
1.7.7.6