2012-03-07 14:50:25

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v2 0/4] client: parameter naming convention

From: Mikel Astiz <[email protected]>

These patches review the naming convention in transfer.h and session.h, in order to avoid misleading use of the terms "name", "filename" and "targetname".

This second proposal splits the first patch into two patches, and avoids a bug that would have been introduced: the reply of the agent needs to be handled with care, since depending on the operation type (put or get) the reply of the agent should overwrite one field or the other.

Mikel Astiz (4):
client: add obc_transfer_set_filename
client: fix naming convention in transfer api
client: refactor naming convention in session api
client: simplify obc_session_pull

client/session.c | 55 ++++++++++++++++++----------------------------------
client/session.h | 12 +++++-----
client/transfer.c | 14 +++++++++---
client/transfer.h | 2 +
4 files changed, 37 insertions(+), 46 deletions(-)

--
1.7.6.5



2012-03-09 00:01:53

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH obexd v2 0/4] client: parameter naming convention

Hi Mikel,

On Wed, Mar 07, 2012, Mikel Astiz wrote:
> From: Mikel Astiz <[email protected]>
>
> These patches review the naming convention in transfer.h and
> session.h, in order to avoid misleading use of the terms "name",
> "filename" and "targetname".
>
> This second proposal splits the first patch into two patches, and
> avoids a bug that would have been introduced: the reply of the agent
> needs to be handled with care, since depending on the operation type
> (put or get) the reply of the agent should overwrite one field or the
> other.
>
> Mikel Astiz (4):
> client: add obc_transfer_set_filename
> client: fix naming convention in transfer api
> client: refactor naming convention in session api
> client: simplify obc_session_pull
>
> client/session.c | 55 ++++++++++++++++++----------------------------------
> client/session.h | 12 +++++-----
> client/transfer.c | 14 +++++++++---
> client/transfer.h | 2 +
> 4 files changed, 37 insertions(+), 46 deletions(-)

All patches in this set have been applied. Thanks.

Johan

2012-03-08 10:28:21

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [PATCH obexd v2 0/4] client: parameter naming convention

Hi Mikel,

On Wed, Mar 7, 2012 at 4:50 PM, Mikel Astiz <[email protected]> wrote:
> From: Mikel Astiz <[email protected]>
>
> These patches review the naming convention in transfer.h and session.h, in order to avoid misleading use of the terms "name", "filename" and "targetname".
>
> This second proposal splits the first patch into two patches, and avoids a bug that would have been introduced: the reply of the agent needs to be handled with care, since depending on the operation type (put or get) the reply of the agent should overwrite one field or the other.
>
> Mikel Astiz (4):
> ?client: add obc_transfer_set_filename
> ?client: fix naming convention in transfer api
> ?client: refactor naming convention in session api
> ?client: simplify obc_session_pull
>
> ?client/session.c ?| ? 55 ++++++++++++++++++----------------------------------
> ?client/session.h ?| ? 12 +++++-----
> ?client/transfer.c | ? 14 +++++++++---
> ?client/transfer.h | ? ?2 +
> ?4 files changed, 37 insertions(+), 46 deletions(-)
>
> --
> 1.7.6.5

Ack.

--
Luiz Augusto von Dentz

2012-03-07 14:50:29

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v2 4/4] client: simplify obc_session_pull

From: Mikel Astiz <[email protected]>

Functions obc_session_get and obc_session_pull nearly share the same
code, so the later can be achieved by just calling the first one.

The session api is not modified in this patch.
---
client/session.c | 22 +---------------------
1 files changed, 1 insertions(+), 21 deletions(-)

diff --git a/client/session.c b/client/session.c
index e130427..a9883c7 100644
--- a/client/session.c
+++ b/client/session.c
@@ -1005,27 +1005,7 @@ int obc_session_pull(struct obc_session *session,
const char *type, const char *targetfile,
session_callback_t function, void *user_data)
{
- struct obc_transfer *transfer;
- const char *agent;
-
- if (session->obex == NULL)
- return -ENOTCONN;
-
- if (session->agent != NULL)
- agent = obc_agent_get_name(session->agent);
- else
- agent = NULL;
-
- transfer = obc_transfer_register(session->conn, session->obex,
- agent,
- targetfile,
- NULL, type,
- NULL);
- if (transfer == NULL) {
- return -EIO;
- }
-
- return session_request(session, transfer, session_prepare_get,
+ return obc_session_get(session, type, NULL, targetfile, NULL, 0,
function, user_data);
}

--
1.7.6.5


2012-03-07 14:50:28

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v2 3/4] client: refactor naming convention in session api

From: Mikel Astiz <[email protected]>

The terms "name", "filename" and "targetname" are used in session.h that
can be confusing. This patch proposes to follow the terminology in the
D-Bus api:
- Name: the remote name of the object being transferred
- Filename: the local filesystem name of a file being sent
- Targetfile: the local filesystem name of a file being
received
---
client/session.c | 29 ++++++++++++++---------------
client/session.h | 12 ++++++------
2 files changed, 20 insertions(+), 21 deletions(-)

diff --git a/client/session.c b/client/session.c
index 28fae03..e130427 100644
--- a/client/session.c
+++ b/client/session.c
@@ -935,7 +935,7 @@ static void session_prepare_get(gpointer data, gpointer user_data)
}

int obc_session_get(struct obc_session *session, const char *type,
- const char *filename, const char *targetname,
+ const char *name, const char *targetfile,
const guint8 *apparam, gint apparam_size,
session_callback_t func, void *user_data)
{
@@ -959,9 +959,8 @@ int obc_session_get(struct obc_session *session, const char *type,
agent = NULL;

transfer = obc_transfer_register(session->conn, session->obex,
- agent, targetname,
- filename, type,
- params);
+ agent, targetfile,
+ name, type, params);
if (transfer == NULL) {
if (params != NULL) {
g_free(params->data);
@@ -975,7 +974,7 @@ int obc_session_get(struct obc_session *session, const char *type,
}

int obc_session_send(struct obc_session *session, const char *filename,
- const char *targetname)
+ const char *name)
{
struct obc_transfer *transfer;
const char *agent;
@@ -988,8 +987,7 @@ int obc_session_send(struct obc_session *session, const char *filename,

transfer = obc_transfer_register(session->conn, session->obex,
agent, filename,
- targetname, NULL,
- NULL);
+ name, NULL, NULL);
if (transfer == NULL)
return -EINVAL;

@@ -1004,7 +1002,7 @@ int obc_session_send(struct obc_session *session, const char *filename,
}

int obc_session_pull(struct obc_session *session,
- const char *type, const char *filename,
+ const char *type, const char *targetfile,
session_callback_t function, void *user_data)
{
struct obc_transfer *transfer;
@@ -1019,7 +1017,8 @@ int obc_session_pull(struct obc_session *session,
agent = NULL;

transfer = obc_transfer_register(session->conn, session->obex,
- agent, filename,
+ agent,
+ targetfile,
NULL, type,
NULL);
if (transfer == NULL) {
@@ -1080,7 +1079,7 @@ static void session_prepare_put(gpointer data, gpointer user_data)
DBG("Transfer(%p) started", transfer);
}

-int obc_session_put(struct obc_session *session, char *buf, const char *targetname)
+int obc_session_put(struct obc_session *session, char *buf, const char *name)
{
struct obc_transfer *transfer;
const char *agent;
@@ -1094,7 +1093,7 @@ int obc_session_put(struct obc_session *session, char *buf, const char *targetna

transfer = obc_transfer_register(session->conn, session->obex,
agent, NULL,
- targetname, NULL,
+ name, NULL,
NULL);
if (transfer == NULL) {
g_free(buf);
@@ -1387,7 +1386,7 @@ guint obc_session_mkdir(struct obc_session *session, const char *folder,
return p->id;
}

-guint obc_session_copy(struct obc_session *session, const char *filename,
+guint obc_session_copy(struct obc_session *session, const char *srcname,
const char *destname, session_callback_t func,
void *user_data, GError **err)
{
@@ -1406,7 +1405,7 @@ guint obc_session_copy(struct obc_session *session, const char *filename,

p = pending_request_new(session, NULL, NULL, func, user_data);

- p->req_id = g_obex_copy(session->obex, filename, destname, async_cb, p,
+ p->req_id = g_obex_copy(session->obex, srcname, destname, async_cb, p,
err);
if (*err != NULL) {
pending_request_free(p);
@@ -1417,7 +1416,7 @@ guint obc_session_copy(struct obc_session *session, const char *filename,
return p->id;
}

-guint obc_session_move(struct obc_session *session, const char *filename,
+guint obc_session_move(struct obc_session *session, const char *srcname,
const char *destname, session_callback_t func,
void *user_data, GError **err)
{
@@ -1436,7 +1435,7 @@ guint obc_session_move(struct obc_session *session, const char *filename,

p = pending_request_new(session, NULL, NULL, func, user_data);

- p->req_id = g_obex_move(session->obex, filename, destname, async_cb, p,
+ p->req_id = g_obex_move(session->obex, srcname, destname, async_cb, p,
err);
if (*err != NULL) {
pending_request_free(p);
diff --git a/client/session.h b/client/session.h
index a19b31f..4bfb41d 100644
--- a/client/session.h
+++ b/client/session.h
@@ -56,18 +56,18 @@ 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);

int obc_session_send(struct obc_session *session, const char *filename,
- const char *remotename);
+ const char *name);
int obc_session_get(struct obc_session *session, const char *type,
- const char *filename, const char *targetname,
+ const char *name, const char *targetfile,
const guint8 *apparam, gint apparam_size,
session_callback_t func, void *user_data);
int obc_session_pull(struct obc_session *session,
- const char *type, const char *filename,
+ const char *type, const char *targetfile,
session_callback_t function, void *user_data);
const char *obc_session_register(struct obc_session *session,
GDBusDestroyFunction destroy);
int obc_session_put(struct obc_session *session, char *buf,
- const char *targetname);
+ const char *name);

guint obc_session_setpath(struct obc_session *session, const char *path,
session_callback_t func, void *user_data,
@@ -75,10 +75,10 @@ guint obc_session_setpath(struct obc_session *session, const char *path,
guint obc_session_mkdir(struct obc_session *session, const char *folder,
session_callback_t func, void *user_data,
GError **err);
-guint obc_session_copy(struct obc_session *session, const char *filename,
+guint obc_session_copy(struct obc_session *session, const char *srcname,
const char *destname, session_callback_t func,
void *user_data, GError **err);
-guint obc_session_move(struct obc_session *session, const char *filename,
+guint obc_session_move(struct obc_session *session, const char *srcname,
const char *destname, session_callback_t func,
void *user_data, GError **err);
guint obc_session_delete(struct obc_session *session, const char *file,
--
1.7.6.5


2012-03-07 14:50:27

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v2 2/4] client: fix naming convention in transfer api

From: Mikel Astiz <[email protected]>

The terms can be quite misleading, so this patch proposes to follow the
terminology in the D-Bus api:
- Name: the remote name of the object being transferred
- Filename: the name of the file in local the filesystem

Both values can be NULL independently.

This fixes the problem of using the terms differently in get and put
operations. The result was that the properties "Name" and "Filename" were
swapped in D-Bus in the case of get.

Once the fields map to obex fields, the interpretation of the response
from the agent becomes more complicated. Depending on the transfer type,
either the name or the filename field must be updated.
---
client/session.c | 16 ++++++++++------
client/transfer.c | 7 +++----
2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/client/session.c b/client/session.c
index 57a5b06..28fae03 100644
--- a/client/session.c
+++ b/client/session.c
@@ -685,8 +685,12 @@ static void session_request_reply(DBusPendingCall *call, gpointer user_data)

DBG("Agent.Request() reply: %s", name);

- if (strlen(name))
- obc_transfer_set_name(transfer, name);
+ if (strlen(name)) {
+ if (p->auth_complete == session_prepare_put)
+ obc_transfer_set_name(transfer, name);
+ else
+ obc_transfer_set_filename(transfer, name);
+ }

if (p->auth_complete)
p->auth_complete(session, transfer);
@@ -955,8 +959,8 @@ int obc_session_get(struct obc_session *session, const char *type,
agent = NULL;

transfer = obc_transfer_register(session->conn, session->obex,
- agent, filename,
- targetname, type,
+ agent, targetname,
+ filename, type,
params);
if (transfer == NULL) {
if (params != NULL) {
@@ -1015,8 +1019,8 @@ int obc_session_pull(struct obc_session *session,
agent = NULL;

transfer = obc_transfer_register(session->conn, session->obex,
- agent, NULL,
- filename, type,
+ agent, filename,
+ NULL, type,
NULL);
if (transfer == NULL) {
return -EIO;
diff --git a/client/transfer.c b/client/transfer.c
index 3471655..60c2e4f 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -526,9 +526,8 @@ int obc_transfer_get(struct obc_transfer *transfer)
strncmp(transfer->type, "x-bt/", 5) == 0)) {
rsp_cb = get_buf_xfer_progress;
} else {
- int fd = open(transfer->name ? : transfer->filename,
+ int fd = open(transfer->filename ? : transfer->name,
O_WRONLY | O_CREAT, 0600);
-
if (fd < 0) {
error("open(): %s(%d)", strerror(errno), errno);
return -errno;
@@ -540,9 +539,9 @@ int obc_transfer_get(struct obc_transfer *transfer)

req = g_obex_packet_new(G_OBEX_OP_GET, TRUE, G_OBEX_HDR_INVALID);

- if (transfer->filename != NULL)
+ if (transfer->name != NULL)
g_obex_packet_add_unicode(req, G_OBEX_HDR_NAME,
- transfer->filename);
+ transfer->name);

if (transfer->type != NULL)
g_obex_packet_add_bytes(req, G_OBEX_HDR_TYPE, transfer->type,
--
1.7.6.5


2012-03-07 14:50:26

by Mikel Astiz

[permalink] [raw]
Subject: [PATCH obexd v2 1/4] client: add obc_transfer_set_filename

From: Mikel Astiz <[email protected]>

This is just a setter for the filename field in transfers.
---
client/transfer.c | 7 +++++++
client/transfer.h | 2 ++
2 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/client/transfer.c b/client/transfer.c
index d1edef2..3471655 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -645,6 +645,13 @@ 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)
+{
+ g_free(transfer->filename);
+ transfer->filename = g_strdup(filename);
+}
+
const char *obc_transfer_get_path(struct obc_transfer *transfer)
{
return transfer->path;
diff --git a/client/transfer.h b/client/transfer.h
index e7e1000..da7d151 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -56,6 +56,8 @@ 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,
+ const char *filename);
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.6.5