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
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
On Thu, 2016-06-30 at 17:01 -0400, Don Zickus wrote:
> the code, it is obvious the funciton is not implemented.
"function"
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
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 <[email protected]>
---
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
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 <[email protected]>.
---
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
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 <[email protected]> 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
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 <[email protected]> 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
Hi Don,
On Fri, Jul 1, 2016 at 12:01 AM, Don Zickus <[email protected]> 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