[PATCH 0/3] obexd fixes

[PATCH 0/3] obexd fixes

[Don Zickus]

We started running some obexd tests to help mimic Fedora users passing info
from their phone to their laptop.  These tests uncovered some issues.

Let me know if I need to fix these patches or try something different.

Cheers,
Don

Don Zickus (3):
  obexd: Allow CreateFolder to create a directory
  obexd: Return dummy_data instead of int in phonebook-dummy
  obexd: Add a detailed failure message for exchanging business cards

 obexd/client/opp.c              |  2 +-
 obexd/plugins/ftp.c             |  2 ++
 obexd/plugins/phonebook-dummy.c | 11 +++++++----
 3 files changed, 10 insertions(+), 5 deletions(-)

-- 
1.8.3.1

[PATCH 1/3] obexd: Allow CreateFolder to create a directory

[Don Zickus]

When the remote device sends the 'CreateFolder' command, obexd
first tries to verify the path in ftp_setpath().  Because we are
creating a new directory, the verify_path() is expected to fail (it does
not exist yet; ENOENT).

Trap that special case and cause the function to fail directly to the
create directory path.

This patch is from Marek Kasik <mkasik@redhat.com>.
---
 obexd/plugins/ftp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/obexd/plugins/ftp.c b/obexd/plugins/ftp.c
index a906527..3ee18a6 100644
--- a/obexd/plugins/ftp.c
+++ b/obexd/plugins/ftp.c
@@ -278,6 +278,8 @@ int ftp_setpath(struct obex_session *os, void *user_data)
 	DBG("Fullname: %s", fullname);
 
 	err = verify_path(fullname);
+	if (err == -ENOENT)
+		goto not_found;
 
 	if (err < 0)
 		goto done;
-- 
1.8.3.1

[PATCH 2/3] obexd: Return dummy_data instead of int in phonebook-dummy

[Don Zickus]

There are two functions in phonebook-dummy that were returning
'int's instead of 'struct dummy_data'

phonebook_create_cache
phonebook_get_entry

As a result, when an obex-client sends the GetSize command, the obexd
on the server segfaults.

Fix this by storing the id and returning the dummy_data struct.

The GetSize test now passes correctly.

Patch from Marek Kasik <mkasik@redhat.com>
---
 obexd/plugins/phonebook-dummy.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/obexd/plugins/phonebook-dummy.c b/obexd/plugins/phonebook-dummy.c
index eeb078f..9ad9cac 100644
--- a/obexd/plugins/phonebook-dummy.c
+++ b/obexd/plugins/phonebook-dummy.c
@@ -538,13 +538,13 @@ void *phonebook_get_entry(const char *folder, const char *id,
 	dummy->apparams = params;
 	dummy->fd = fd;
 
-	ret = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, read_entry, dummy,
+	dummy->id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, read_entry, dummy,
 								dummy_free);
 
 	if (err)
 		*err = 0;
 
-	return GINT_TO_POINTER(ret);
+	return dummy;
 }
 
 void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
@@ -554,6 +554,7 @@ void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 	char *foldername;
 	DIR *dp;
 	guint ret;
+	struct dummy_data *dummy;
 
 	foldername = g_build_filename(root_folder, name, NULL);
 	dp = opendir(foldername);
@@ -572,11 +573,13 @@ void *phonebook_create_cache(const char *name, phonebook_entry_cb entry_cb,
 	query->user_data = user_data;
 	query->dp = dp;
 
-	ret = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, create_cache, query,
+	dummy = g_new0(struct dummy_data, 1);
+
+	dummy->id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, create_cache, query,
 								query_free);
 
 	if (err)
 		*err = 0;
 
-	return GINT_TO_POINTER(ret);
+	return dummy;
 }
-- 
1.8.3.1

[PATCH 3/3] obexd: Add a detailed failure message for exchanging business cards

[Don Zickus]

When sending the ExchangeBusinessCards() command, the command returns
a failure.  It isn't clear what that failure is.  Upon looking through
the code, it is obvious the funciton is not implemented.

This patch just adds an extra detail message 'Not Implemented' to make
the failure a little more clear about what the problem is.
---
 obexd/client/opp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/obexd/client/opp.c b/obexd/client/opp.c
index 3c2801a..0e063dd 100644
--- a/obexd/client/opp.c
+++ b/obexd/client/opp.c
@@ -117,7 +117,7 @@ fail:
 static DBusMessage *opp_exchange_business_cards(DBusConnection *connection,
 					DBusMessage *message, void *user_data)
 {
-	return g_dbus_create_error(message, ERROR_INTERFACE ".Failed", NULL);
+	return g_dbus_create_error(message, ERROR_INTERFACE ".Failed", "Not Implemented");
 }
 
 static const GDBusMethodTable opp_methods[] = {
-- 
1.8.3.1

Re: [PATCH 3/3] obexd: Add a detailed failure message for exchanging business cards

[Bastien Nocera]

On Thu, 2016-06-30 at 17:01 -0400, Don Zickus wrote:
> the code, it is obvious the funciton is not implemented.

"function"

Re: [PATCH 3/3] obexd: Add a detailed failure message for exchanging business cards

[Don Zickus]

On Thu, Jun 30, 2016 at 11:35:13PM +0200, Bastien Nocera wrote:
> On Thu, 2016-06-30 at 17:01 -0400, Don Zickus wrote:
> > the code, it is obvious the funciton is not implemented.
> 
> "function"

doh.  Thanks!

Cheers,
Don

Re: [PATCH 0/3] obexd fixes

[Luiz Augusto von Dentz]

Hi Don,

On Fri, Jul 1, 2016 at 12:01 AM, Don Zickus <dzickus@redhat.com> wrote:
> We started running some obexd tests to help mimic Fedora users passing info
> from their phone to their laptop.  These tests uncovered some issues.
>
> Let me know if I need to fix these patches or try something different.
>
> Cheers,
> Don
>
> Don Zickus (3):
>   obexd: Allow CreateFolder to create a directory
>   obexd: Return dummy_data instead of int in phonebook-dummy
>   obexd: Add a detailed failure message for exchanging business cards
>
>  obexd/client/opp.c              |  2 +-
>  obexd/plugins/ftp.c             |  2 ++
>  obexd/plugins/phonebook-dummy.c | 11 +++++++----
>  3 files changed, 10 insertions(+), 5 deletions(-)
>
> --
> 1.8.3.1

Applied after fixing the following problems:

#36: FILE: obexd/plugins/phonebook-dummy.c:578:
+ dummy->id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, create_cache, query,

obexd/plugins/phonebook-dummy.c:523:8: error: unused variable ‘ret’
[-Werror=unused-variable]
  guint ret;
        ^~~
obexd/plugins/phonebook-dummy.c:556:8: error: unused variable ‘ret’
[-Werror=unused-variable]
  guint ret;
        ^~~

Ive also fixed the author of the patches so we don't have to add in
the patch description.

-- 
Luiz Augusto von Dentz

Re: [PATCH 0/3] obexd fixes

[Don Zickus]

On Mon, Jul 04, 2016 at 03:07:47PM +0300, Luiz Augusto von Dentz wrote:
> Hi Don,
> 
> On Fri, Jul 1, 2016 at 12:01 AM, Don Zickus <dzickus@redhat.com> wrote:
> > We started running some obexd tests to help mimic Fedora users passing info
> > from their phone to their laptop.  These tests uncovered some issues.
> >
> > Let me know if I need to fix these patches or try something different.
> >
> > Cheers,
> > Don
> >
> > Don Zickus (3):
> >   obexd: Allow CreateFolder to create a directory
> >   obexd: Return dummy_data instead of int in phonebook-dummy
> >   obexd: Add a detailed failure message for exchanging business cards
> >
> >  obexd/client/opp.c              |  2 +-
> >  obexd/plugins/ftp.c             |  2 ++
> >  obexd/plugins/phonebook-dummy.c | 11 +++++++----
> >  3 files changed, 10 insertions(+), 5 deletions(-)
> >
> > --
> > 1.8.3.1
> 
> Applied after fixing the following problems:
> 
> #36: FILE: obexd/plugins/phonebook-dummy.c:578:
> + dummy->id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, create_cache, query,
> 
> obexd/plugins/phonebook-dummy.c:523:8: error: unused variable ‘ret’
> [-Werror=unused-variable]
>   guint ret;
>         ^~~
> obexd/plugins/phonebook-dummy.c:556:8: error: unused variable ‘ret’
> [-Werror=unused-variable]
>   guint ret;
>         ^~~

Awesome!  Thanks.  Sorry about the warning.

> 
> Ive also fixed the author of the patches so we don't have to add in
> the patch description.

Perfect.  Yeah, Marek should take full credit for those patches.

Cheers,
Don

Re: [PATCH 0/3] obexd fixes

[Don Zickus]

On Mon, Jul 04, 2016 at 03:07:47PM +0300, Luiz Augusto von Dentz wrote:
> Hi Don,
> 
> On Fri, Jul 1, 2016 at 12:01 AM, Don Zickus <dzickus@redhat.com> wrote:
> > We started running some obexd tests to help mimic Fedora users passing info
> > from their phone to their laptop.  These tests uncovered some issues.
> >
> > Let me know if I need to fix these patches or try something different.
> >
> > Cheers,
> > Don
> >
> > Don Zickus (3):
> >   obexd: Allow CreateFolder to create a directory
> >   obexd: Return dummy_data instead of int in phonebook-dummy
> >   obexd: Add a detailed failure message for exchanging business cards
> >
> >  obexd/client/opp.c              |  2 +-
> >  obexd/plugins/ftp.c             |  2 ++
> >  obexd/plugins/phonebook-dummy.c | 11 +++++++----
> >  3 files changed, 10 insertions(+), 5 deletions(-)
> >
> > --
> > 1.8.3.1
> 
> Applied after fixing the following problems:
> 
> #36: FILE: obexd/plugins/phonebook-dummy.c:578:
> + dummy->id = g_idle_add_full(G_PRIORITY_DEFAULT_IDLE, create_cache, query,
> 
> obexd/plugins/phonebook-dummy.c:523:8: error: unused variable ‘ret’
> [-Werror=unused-variable]
>   guint ret;
>         ^~~
> obexd/plugins/phonebook-dummy.c:556:8: error: unused variable ‘ret’
> [-Werror=unused-variable]
>   guint ret;
>         ^~~
> 
> Ive also fixed the author of the patches so we don't have to add in
> the patch description.

Hi Luiz,

Side question,  Marek posted a patch back in April that didn't get any
attention.  It seems you are the committer for that code??  Could you look
and provide some feedback, if you have time?

http://article.gmane.org/gmane.linux.bluez.kernel/67420/match=

Thanks!

Cheers,
Don