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

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

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

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

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

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

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

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

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

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

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

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

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

[PATCH obexd v2 4/4] client: simplify obc_session_pull

[Mikel Astiz]

From: Mikel Astiz <mikel.astiz@bmw-carit.de>

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

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

[Luiz Augusto von Dentz]

Hi Mikel,

On Wed, Mar 7, 2012 at 4:50 PM, Mikel Astiz <mikel.astiz.oss@gmail.com> wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
>
> 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

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

[Johan Hedberg]

Hi Mikel,

On Wed, Mar 07, 2012, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz@bmw-carit.de>
> 
> 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