* [PATCH] gobex: Fix setpath to match one from OBEX spec @ 2011-11-15 14:44 Bartosz Szatkowski 2011-11-15 22:29 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 6+ messages in thread From: Bartosz Szatkowski @ 2011-11-15 14:44 UTC (permalink / raw) To: linux-bluetooth; +Cc: Bartosz Szatkowski OBEX spec states that when bit 0 is set in flags, server should backup a level before applying name. It's accomplished by moving parsing of the path to upper layers (as gobex is low level lib and I think it should be based on level defined in OBEX spec, upper profiles may wrap that as defined in correspondent specifications) and exposing *cdup* flag in function parameters. --- client/ftp.c | 6 +++++- client/pbap.c | 4 ++-- gobex/gobex.c | 16 ++++++++++------ gobex/gobex.h | 5 +++-- unit/test-gobex.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 64 insertions(+), 13 deletions(-) diff --git a/client/ftp.c b/client/ftp.c index 336fc26..d9e31ad 100644 --- a/client/ftp.c +++ b/client/ftp.c @@ -82,7 +82,11 @@ static DBusMessage *change_folder(DBusConnection *connection, return g_dbus_create_error(message, "org.openobex.Error.InvalidArguments", NULL); - g_obex_setpath(obex, folder, async_cb, message, &err); + if (g_strcmp0("..", folder) == 0) + g_obex_setpath(obex, NULL, TRUE, async_cb, message, &err); + else + g_obex_setpath(obex, folder, FALSE, async_cb, message, &err); + if (err != NULL) { DBusMessage *reply; reply = g_dbus_create_error(message, diff --git a/client/pbap.c b/client/pbap.c index 9e9eb05..5055ad9 100644 --- a/client/pbap.c +++ b/client/pbap.c @@ -277,7 +277,7 @@ static void setpath_cb(GObex *obex, GError *err, GObexPacket *rsp, data->index++; - g_obex_setpath(obex, next, setpath_cb, data, &err); + g_obex_setpath(obex, next, FALSE, setpath_cb, data, &err); if (err != NULL) { setpath_complete(err, data); g_error_free(err); @@ -292,7 +292,7 @@ static gboolean setpath(GObex *obex, const char *path, size_t max_elem, data = g_new0(struct setpath_data, 1); - g_obex_setpath(obex, "", setpath_cb, data, &err); + g_obex_setpath(obex, NULL, FALSE, setpath_cb, data, &err); if (err != NULL) { error("set_path: %s", err->message); g_error_free(err); diff --git a/gobex/gobex.c b/gobex/gobex.c index 7840304..1085e78 100644 --- a/gobex/gobex.c +++ b/gobex/gobex.c @@ -1083,11 +1083,13 @@ guint g_obex_connect(GObex *obex, GObexResponseFunc func, gpointer user_data, return g_obex_send_req(obex, req, -1, func, user_data, err); } -guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, - gpointer user_data, GError **err) +guint g_obex_setpath(GObex *obex, const char *folder, gboolean cdup, + GObexResponseFunc func, gpointer user_data, + GError **err) { GObexPacket *req; struct setpath_data data; + GObexHeader *hdr; g_obex_debug(G_OBEX_DEBUG_COMMAND, "conn %u", obex->conn_id); @@ -1095,12 +1097,14 @@ guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, memset(&data, 0, sizeof(data)); - if (strcmp(path, "..") == 0) + if (cdup) data.flags = 0x03; - else { - GObexHeader *hdr; + else data.flags = 0x02; - hdr = g_obex_header_new_unicode(G_OBEX_HDR_NAME, path); + + if (!cdup || (folder != NULL && folder[0] != '\0')) { + hdr = g_obex_header_new_unicode(G_OBEX_HDR_NAME, + folder != NULL ? folder : ""); g_obex_packet_add_header(req, hdr); } diff --git a/gobex/gobex.h b/gobex/gobex.h index 1b20333..d9d470d 100644 --- a/gobex/gobex.h +++ b/gobex/gobex.h @@ -73,8 +73,9 @@ void g_obex_unref(GObex *obex); guint g_obex_connect(GObex *obex, GObexResponseFunc func, gpointer user_data, GError **err, guint8 first_hdr_id, ...); -guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, - gpointer user_data, GError **err); +guint g_obex_setpath(GObex *obex, const char *folder, gboolean cdup, + GObexResponseFunc func, gpointer user_data, + GError **err); guint g_obex_mkdir(GObex *obex, const char *path, GObexResponseFunc func, gpointer user_data, GError **err); diff --git a/unit/test-gobex.c b/unit/test-gobex.c index 62443db..8b86dda 100644 --- a/unit/test-gobex.c +++ b/unit/test-gobex.c @@ -46,6 +46,10 @@ static uint8_t pkt_setpath_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, 0x00, 0x10, 0, 'd', 0, 'i', 0, 'r', 0, 0 }; static uint8_t pkt_setpath_up_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, 0x00, 0x05, 0x03, 0x00 }; +static uint8_t pkt_setpath_up_down_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, + 0x00, 0x10, 0x03, 0x00, + G_OBEX_HDR_NAME, 0x00, 0x0b, + 0, 'd', 0, 'i', 0, 'r', 0, 0 }; static uint8_t pkt_success_rsp[] = { 0x20 | FINAL_BIT, 0x00, 0x03 }; static uint8_t pkt_mkdir_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, 0x00, 0x10, @@ -890,7 +894,7 @@ static void test_setpath(void) timer_id = g_timeout_add_seconds(1, test_timeout, &d); - g_obex_setpath(obex, "dir", req_complete, &d, &d.err); + g_obex_setpath(obex, "dir", FALSE, req_complete, &d, &d.err); g_assert_no_error(d.err); g_main_loop_run(d.mainloop); @@ -926,7 +930,44 @@ static void test_setpath_up(void) timer_id = g_timeout_add_seconds(1, test_timeout, &d); - g_obex_setpath(obex, "..", req_complete, &d, &d.err); + g_obex_setpath(obex, "", TRUE, req_complete, &d, &d.err); + g_assert_no_error(d.err); + + g_main_loop_run(d.mainloop); + + g_assert_cmpuint(d.count, ==, 1); + + g_main_loop_unref(d.mainloop); + + g_source_remove(timer_id); + g_io_channel_unref(io); + g_source_remove(io_id); + g_obex_unref(obex); + + g_assert_no_error(d.err); +} + +static void test_setpath_up_down(void) +{ + GIOChannel *io; + GIOCondition cond; + guint io_id, timer_id; + GObex *obex; + struct test_data d = { 0, NULL, { + { pkt_setpath_up_down_req, + sizeof(pkt_setpath_up_down_req) } }, { + { pkt_success_rsp, sizeof(pkt_success_rsp) } } }; + + create_endpoints(&obex, &io, SOCK_STREAM); + + cond = G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL; + io_id = g_io_add_watch(io, cond, test_io_cb, &d); + + d.mainloop = g_main_loop_new(NULL, FALSE); + + timer_id = g_timeout_add_seconds(1, test_timeout, &d); + + g_obex_setpath(obex, "dir", TRUE, req_complete, &d, &d.err); g_assert_no_error(d.err); g_main_loop_run(d.mainloop); @@ -1142,6 +1183,7 @@ int main(int argc, char *argv[]) g_test_add_func("/gobex/test_setpath", test_setpath); g_test_add_func("/gobex/test_setpath_up", test_setpath_up); + g_test_add_func("/gobex/test_setpath_up_down", test_setpath_up_down); g_test_add_func("/gobex/test_mkdir", test_mkdir); -- 1.7.4.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] gobex: Fix setpath to match one from OBEX spec 2011-11-15 14:44 [PATCH] gobex: Fix setpath to match one from OBEX spec Bartosz Szatkowski @ 2011-11-15 22:29 ` Luiz Augusto von Dentz 2011-11-16 8:24 ` Bartosz Szatkowski 0 siblings, 1 reply; 6+ messages in thread From: Luiz Augusto von Dentz @ 2011-11-15 22:29 UTC (permalink / raw) To: Bartosz Szatkowski; +Cc: linux-bluetooth Hi Bartosz, On Tue, Nov 15, 2011 at 4:44 PM, Bartosz Szatkowski <bulislaw@linux.com> wrote: > OBEX spec states that when bit 0 is set in flags, server should backup a > level before applying name. It's accomplished by moving parsing of the > path to upper layers (as gobex is low level lib and I think it should be > based on level defined in OBEX spec, upper profiles may wrap that as > defined in correspondent specifications) and exposing *cdup* flag in > function parameters. > --- > client/ftp.c | 6 +++++- > client/pbap.c | 4 ++-- > gobex/gobex.c | 16 ++++++++++------ > gobex/gobex.h | 5 +++-- > unit/test-gobex.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- > 5 files changed, 64 insertions(+), 13 deletions(-) > > diff --git a/client/ftp.c b/client/ftp.c > index 336fc26..d9e31ad 100644 > --- a/client/ftp.c > +++ b/client/ftp.c > @@ -82,7 +82,11 @@ static DBusMessage *change_folder(DBusConnection *connection, > return g_dbus_create_error(message, > "org.openobex.Error.InvalidArguments", NULL); > > - g_obex_setpath(obex, folder, async_cb, message, &err); > + if (g_strcmp0("..", folder) == 0) > + g_obex_setpath(obex, NULL, TRUE, async_cb, message, &err); > + else > + g_obex_setpath(obex, folder, FALSE, async_cb, message, &err); > + > if (err != NULL) { > DBusMessage *reply; > reply = g_dbus_create_error(message, > diff --git a/client/pbap.c b/client/pbap.c > index 9e9eb05..5055ad9 100644 > --- a/client/pbap.c > +++ b/client/pbap.c > @@ -277,7 +277,7 @@ static void setpath_cb(GObex *obex, GError *err, GObexPacket *rsp, > > data->index++; > > - g_obex_setpath(obex, next, setpath_cb, data, &err); > + g_obex_setpath(obex, next, FALSE, setpath_cb, data, &err); > if (err != NULL) { > setpath_complete(err, data); > g_error_free(err); > @@ -292,7 +292,7 @@ static gboolean setpath(GObex *obex, const char *path, size_t max_elem, > > data = g_new0(struct setpath_data, 1); > > - g_obex_setpath(obex, "", setpath_cb, data, &err); > + g_obex_setpath(obex, NULL, FALSE, setpath_cb, data, &err); > if (err != NULL) { > error("set_path: %s", err->message); > g_error_free(err); > diff --git a/gobex/gobex.c b/gobex/gobex.c > index 7840304..1085e78 100644 > --- a/gobex/gobex.c > +++ b/gobex/gobex.c > @@ -1083,11 +1083,13 @@ guint g_obex_connect(GObex *obex, GObexResponseFunc func, gpointer user_data, > return g_obex_send_req(obex, req, -1, func, user_data, err); > } > > -guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, > - gpointer user_data, GError **err) > +guint g_obex_setpath(GObex *obex, const char *folder, gboolean cdup, > + GObexResponseFunc func, gpointer user_data, > + GError **err) > { > GObexPacket *req; > struct setpath_data data; > + GObexHeader *hdr; > > g_obex_debug(G_OBEX_DEBUG_COMMAND, "conn %u", obex->conn_id); > > @@ -1095,12 +1097,14 @@ guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, > > memset(&data, 0, sizeof(data)); > > - if (strcmp(path, "..") == 0) > + if (cdup) > data.flags = 0x03; > - else { > - GObexHeader *hdr; > + else > data.flags = 0x02; > - hdr = g_obex_header_new_unicode(G_OBEX_HDR_NAME, path); > + > + if (!cdup || (folder != NULL && folder[0] != '\0')) { > + hdr = g_obex_header_new_unicode(G_OBEX_HDR_NAME, > + folder != NULL ? folder : ""); > g_obex_packet_add_header(req, hdr); > } > > diff --git a/gobex/gobex.h b/gobex/gobex.h > index 1b20333..d9d470d 100644 > --- a/gobex/gobex.h > +++ b/gobex/gobex.h > @@ -73,8 +73,9 @@ void g_obex_unref(GObex *obex); > guint g_obex_connect(GObex *obex, GObexResponseFunc func, gpointer user_data, > GError **err, guint8 first_hdr_id, ...); > > -guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, > - gpointer user_data, GError **err); > +guint g_obex_setpath(GObex *obex, const char *folder, gboolean cdup, > + GObexResponseFunc func, gpointer user_data, > + GError **err); > > guint g_obex_mkdir(GObex *obex, const char *path, GObexResponseFunc func, > gpointer user_data, GError **err); > diff --git a/unit/test-gobex.c b/unit/test-gobex.c > index 62443db..8b86dda 100644 > --- a/unit/test-gobex.c > +++ b/unit/test-gobex.c > @@ -46,6 +46,10 @@ static uint8_t pkt_setpath_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, 0x00, 0x10, > 0, 'd', 0, 'i', 0, 'r', 0, 0 }; > static uint8_t pkt_setpath_up_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, > 0x00, 0x05, 0x03, 0x00 }; > +static uint8_t pkt_setpath_up_down_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, > + 0x00, 0x10, 0x03, 0x00, > + G_OBEX_HDR_NAME, 0x00, 0x0b, > + 0, 'd', 0, 'i', 0, 'r', 0, 0 }; > static uint8_t pkt_success_rsp[] = { 0x20 | FINAL_BIT, 0x00, 0x03 }; > > static uint8_t pkt_mkdir_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, 0x00, 0x10, > @@ -890,7 +894,7 @@ static void test_setpath(void) > > timer_id = g_timeout_add_seconds(1, test_timeout, &d); > > - g_obex_setpath(obex, "dir", req_complete, &d, &d.err); > + g_obex_setpath(obex, "dir", FALSE, req_complete, &d, &d.err); > g_assert_no_error(d.err); > > g_main_loop_run(d.mainloop); > @@ -926,7 +930,44 @@ static void test_setpath_up(void) > > timer_id = g_timeout_add_seconds(1, test_timeout, &d); > > - g_obex_setpath(obex, "..", req_complete, &d, &d.err); > + g_obex_setpath(obex, "", TRUE, req_complete, &d, &d.err); > + g_assert_no_error(d.err); > + > + g_main_loop_run(d.mainloop); > + > + g_assert_cmpuint(d.count, ==, 1); > + > + g_main_loop_unref(d.mainloop); > + > + g_source_remove(timer_id); > + g_io_channel_unref(io); > + g_source_remove(io_id); > + g_obex_unref(obex); > + > + g_assert_no_error(d.err); > +} > + > +static void test_setpath_up_down(void) > +{ > + GIOChannel *io; > + GIOCondition cond; > + guint io_id, timer_id; > + GObex *obex; > + struct test_data d = { 0, NULL, { > + { pkt_setpath_up_down_req, > + sizeof(pkt_setpath_up_down_req) } }, { > + { pkt_success_rsp, sizeof(pkt_success_rsp) } } }; > + > + create_endpoints(&obex, &io, SOCK_STREAM); > + > + cond = G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL; > + io_id = g_io_add_watch(io, cond, test_io_cb, &d); > + > + d.mainloop = g_main_loop_new(NULL, FALSE); > + > + timer_id = g_timeout_add_seconds(1, test_timeout, &d); > + > + g_obex_setpath(obex, "dir", TRUE, req_complete, &d, &d.err); > g_assert_no_error(d.err); > > g_main_loop_run(d.mainloop); > @@ -1142,6 +1183,7 @@ int main(int argc, char *argv[]) > > g_test_add_func("/gobex/test_setpath", test_setpath); > g_test_add_func("/gobex/test_setpath_up", test_setpath_up); > + g_test_add_func("/gobex/test_setpath_up_down", test_setpath_up_down); > > g_test_add_func("/gobex/test_mkdir", test_mkdir); > > -- > 1.7.4.1 I don't understand why you want to carry the burden of operating in non standard paths? Or you are also planning to change the D-Bus API? OBEX spec is just broke in this aspect, it should have allowed standard paths and not this one level + cdup and now you are trying to push this broken concept up to the user, why? How many uis have you seem that can use do cdup + dir in practice? I don't remember seeing any, and if that exist that would probably need to cache the folder listing of the previous folder to be able to present to the user and trust it wont get change in the meantime. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gobex: Fix setpath to match one from OBEX spec 2011-11-15 22:29 ` Luiz Augusto von Dentz @ 2011-11-16 8:24 ` Bartosz Szatkowski 2011-11-16 11:12 ` Luiz Augusto von Dentz 0 siblings, 1 reply; 6+ messages in thread From: Bartosz Szatkowski @ 2011-11-16 8:24 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: linux-bluetooth On Tue, Nov 15, 2011 at 11:29 PM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > Hi Bartosz, > > On Tue, Nov 15, 2011 at 4:44 PM, Bartosz Szatkowski <bulislaw@linux.com> wrote: >> OBEX spec states that when bit 0 is set in flags, server should backup a >> level before applying name. It's accomplished by moving parsing of the >> path to upper layers (as gobex is low level lib and I think it should be >> based on level defined in OBEX spec, upper profiles may wrap that as >> defined in correspondent specifications) and exposing *cdup* flag in >> function parameters. >> --- >> client/ftp.c | 6 +++++- >> client/pbap.c | 4 ++-- >> gobex/gobex.c | 16 ++++++++++------ >> gobex/gobex.h | 5 +++-- >> unit/test-gobex.c | 46 ++++++++++++++++++++++++++++++++++++++++++++-- >> 5 files changed, 64 insertions(+), 13 deletions(-) >> >> diff --git a/client/ftp.c b/client/ftp.c >> index 336fc26..d9e31ad 100644 >> --- a/client/ftp.c >> +++ b/client/ftp.c >> @@ -82,7 +82,11 @@ static DBusMessage *change_folder(DBusConnection *connection, >> return g_dbus_create_error(message, >> "org.openobex.Error.InvalidArguments", NULL); >> >> - g_obex_setpath(obex, folder, async_cb, message, &err); >> + if (g_strcmp0("..", folder) == 0) >> + g_obex_setpath(obex, NULL, TRUE, async_cb, message, &err); >> + else >> + g_obex_setpath(obex, folder, FALSE, async_cb, message, &err); >> + >> if (err != NULL) { >> DBusMessage *reply; >> reply = g_dbus_create_error(message, >> diff --git a/client/pbap.c b/client/pbap.c >> index 9e9eb05..5055ad9 100644 >> --- a/client/pbap.c >> +++ b/client/pbap.c >> @@ -277,7 +277,7 @@ static void setpath_cb(GObex *obex, GError *err, GObexPacket *rsp, >> >> data->index++; >> >> - g_obex_setpath(obex, next, setpath_cb, data, &err); >> + g_obex_setpath(obex, next, FALSE, setpath_cb, data, &err); >> if (err != NULL) { >> setpath_complete(err, data); >> g_error_free(err); >> @@ -292,7 +292,7 @@ static gboolean setpath(GObex *obex, const char *path, size_t max_elem, >> >> data = g_new0(struct setpath_data, 1); >> >> - g_obex_setpath(obex, "", setpath_cb, data, &err); >> + g_obex_setpath(obex, NULL, FALSE, setpath_cb, data, &err); >> if (err != NULL) { >> error("set_path: %s", err->message); >> g_error_free(err); >> diff --git a/gobex/gobex.c b/gobex/gobex.c >> index 7840304..1085e78 100644 >> --- a/gobex/gobex.c >> +++ b/gobex/gobex.c >> @@ -1083,11 +1083,13 @@ guint g_obex_connect(GObex *obex, GObexResponseFunc func, gpointer user_data, >> return g_obex_send_req(obex, req, -1, func, user_data, err); >> } >> >> -guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, >> - gpointer user_data, GError **err) >> +guint g_obex_setpath(GObex *obex, const char *folder, gboolean cdup, >> + GObexResponseFunc func, gpointer user_data, >> + GError **err) >> { >> GObexPacket *req; >> struct setpath_data data; >> + GObexHeader *hdr; >> >> g_obex_debug(G_OBEX_DEBUG_COMMAND, "conn %u", obex->conn_id); >> >> @@ -1095,12 +1097,14 @@ guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, >> >> memset(&data, 0, sizeof(data)); >> >> - if (strcmp(path, "..") == 0) >> + if (cdup) >> data.flags = 0x03; >> - else { >> - GObexHeader *hdr; >> + else >> data.flags = 0x02; >> - hdr = g_obex_header_new_unicode(G_OBEX_HDR_NAME, path); >> + >> + if (!cdup || (folder != NULL && folder[0] != '\0')) { >> + hdr = g_obex_header_new_unicode(G_OBEX_HDR_NAME, >> + folder != NULL ? folder : ""); >> g_obex_packet_add_header(req, hdr); >> } >> >> diff --git a/gobex/gobex.h b/gobex/gobex.h >> index 1b20333..d9d470d 100644 >> --- a/gobex/gobex.h >> +++ b/gobex/gobex.h >> @@ -73,8 +73,9 @@ void g_obex_unref(GObex *obex); >> guint g_obex_connect(GObex *obex, GObexResponseFunc func, gpointer user_data, >> GError **err, guint8 first_hdr_id, ...); >> >> -guint g_obex_setpath(GObex *obex, const char *path, GObexResponseFunc func, >> - gpointer user_data, GError **err); >> +guint g_obex_setpath(GObex *obex, const char *folder, gboolean cdup, >> + GObexResponseFunc func, gpointer user_data, >> + GError **err); >> >> guint g_obex_mkdir(GObex *obex, const char *path, GObexResponseFunc func, >> gpointer user_data, GError **err); >> diff --git a/unit/test-gobex.c b/unit/test-gobex.c >> index 62443db..8b86dda 100644 >> --- a/unit/test-gobex.c >> +++ b/unit/test-gobex.c >> @@ -46,6 +46,10 @@ static uint8_t pkt_setpath_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, 0x00, 0x10, >> 0, 'd', 0, 'i', 0, 'r', 0, 0 }; >> static uint8_t pkt_setpath_up_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, >> 0x00, 0x05, 0x03, 0x00 }; >> +static uint8_t pkt_setpath_up_down_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, >> + 0x00, 0x10, 0x03, 0x00, >> + G_OBEX_HDR_NAME, 0x00, 0x0b, >> + 0, 'd', 0, 'i', 0, 'r', 0, 0 }; >> static uint8_t pkt_success_rsp[] = { 0x20 | FINAL_BIT, 0x00, 0x03 }; >> >> static uint8_t pkt_mkdir_req[] = { G_OBEX_OP_SETPATH | FINAL_BIT, 0x00, 0x10, >> @@ -890,7 +894,7 @@ static void test_setpath(void) >> >> timer_id = g_timeout_add_seconds(1, test_timeout, &d); >> >> - g_obex_setpath(obex, "dir", req_complete, &d, &d.err); >> + g_obex_setpath(obex, "dir", FALSE, req_complete, &d, &d.err); >> g_assert_no_error(d.err); >> >> g_main_loop_run(d.mainloop); >> @@ -926,7 +930,44 @@ static void test_setpath_up(void) >> >> timer_id = g_timeout_add_seconds(1, test_timeout, &d); >> >> - g_obex_setpath(obex, "..", req_complete, &d, &d.err); >> + g_obex_setpath(obex, "", TRUE, req_complete, &d, &d.err); >> + g_assert_no_error(d.err); >> + >> + g_main_loop_run(d.mainloop); >> + >> + g_assert_cmpuint(d.count, ==, 1); >> + >> + g_main_loop_unref(d.mainloop); >> + >> + g_source_remove(timer_id); >> + g_io_channel_unref(io); >> + g_source_remove(io_id); >> + g_obex_unref(obex); >> + >> + g_assert_no_error(d.err); >> +} >> + >> +static void test_setpath_up_down(void) >> +{ >> + GIOChannel *io; >> + GIOCondition cond; >> + guint io_id, timer_id; >> + GObex *obex; >> + struct test_data d = { 0, NULL, { >> + { pkt_setpath_up_down_req, >> + sizeof(pkt_setpath_up_down_req) } }, { >> + { pkt_success_rsp, sizeof(pkt_success_rsp) } } }; >> + >> + create_endpoints(&obex, &io, SOCK_STREAM); >> + >> + cond = G_IO_IN | G_IO_HUP | G_IO_ERR | G_IO_NVAL; >> + io_id = g_io_add_watch(io, cond, test_io_cb, &d); >> + >> + d.mainloop = g_main_loop_new(NULL, FALSE); >> + >> + timer_id = g_timeout_add_seconds(1, test_timeout, &d); >> + >> + g_obex_setpath(obex, "dir", TRUE, req_complete, &d, &d.err); >> g_assert_no_error(d.err); >> >> g_main_loop_run(d.mainloop); >> @@ -1142,6 +1183,7 @@ int main(int argc, char *argv[]) >> >> g_test_add_func("/gobex/test_setpath", test_setpath); >> g_test_add_func("/gobex/test_setpath_up", test_setpath_up); >> + g_test_add_func("/gobex/test_setpath_up_down", test_setpath_up_down); >> >> g_test_add_func("/gobex/test_mkdir", test_mkdir); >> >> -- >> 1.7.4.1 > > I don't understand why you want to carry the burden of operating in > non standard paths? Or you are also planning to change the D-Bus API? > OBEX spec is just broke in this aspect, it should have allowed > standard paths and not this one level + cdup and now you are trying to > push this broken concept up to the user, why? How many uis have you > seem that can use do cdup + dir in practice? I don't remember seeing > any, and if that exist that would probably need to cache the folder > listing of the previous folder to be able to present to the user and > trust it wont get change in the meantime. > > -- > Luiz Augusto von Dentz > Come on did You even read it?? This function is OBEX level (an it works before in similar way - only one dir at a time) - we may don't like specification, but seriously? Just read the patch, i exposed on OBEX level stuff defined in the OBEX specification (as MAP needs operation like "go one level up and than down" in one step) and as it would be possible to just add some "if" for "../dir" in previous version, but it is better to fix broken things than to propagate them (it's obex level function not high level abstraction - deal with it). Each profile may use it multiple times for paths delivered through dbus (but it provokes some questions, that i don't like). Generally there is REALLY many gui that works like that (one level down or up at a time) just look around (eg all file explorers) and it's all you need, because its basically same context! I left profiles alone, just added extra parameter to be ok with new definition. -- Pozdrowienia - Cheers, Bartosz Szatkowski ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gobex: Fix setpath to match one from OBEX spec 2011-11-16 8:24 ` Bartosz Szatkowski @ 2011-11-16 11:12 ` Luiz Augusto von Dentz 2011-11-16 11:49 ` Slawomir Bochenski 0 siblings, 1 reply; 6+ messages in thread From: Luiz Augusto von Dentz @ 2011-11-16 11:12 UTC (permalink / raw) To: Bartosz Szatkowski; +Cc: linux-bluetooth Hi Bartosz, On Wed, Nov 16, 2011 at 10:24 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote: > > Come on did You even read it?? This function is OBEX level (an it > works before in similar way - only one dir at a time) - we may don't > like specification, but seriously? Just read the patch, i exposed on > OBEX level stuff defined in the OBEX specification (as MAP needs > operation like "go one level up and than down" in one step) and as it > would be possible to just add some "if" for "../dir" in previous > version, but it is better to fix broken things than to propagate them > (it's obex level function not high level abstraction - deal with it). Yeah, lets create an API that we don't use, really? Your MAP API need it but that doesn't mean we agree on using it as it is, that table you use to explain how it works is already a sign that it is over-engineered and will be easily misinterpreted by applications. We will not break MAP by not exposing this complexity since we can still do it transparently, even doing cdup and another SetPath although inefficient would still work. > Each profile may use it multiple times for paths delivered through > dbus (but it provokes some questions, that i don't like). Generally > there is REALLY many gui that works like that (one level down or up at > a time) just look around (eg all file explorers) and it's all you > need, because its basically same context! Same context? What context, the folder listing is per folder, besides nowadays the file explores have abandon the tree view in favor of more simple one level like nautilus. But even with gui that uses tree view we can only jump one level or to the root so it is not that it would save that much of time. -- Luiz Augusto von Dentz ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gobex: Fix setpath to match one from OBEX spec 2011-11-16 11:12 ` Luiz Augusto von Dentz @ 2011-11-16 11:49 ` Slawomir Bochenski 2011-11-16 12:00 ` Johan Hedberg 0 siblings, 1 reply; 6+ messages in thread From: Slawomir Bochenski @ 2011-11-16 11:49 UTC (permalink / raw) To: Luiz Augusto von Dentz; +Cc: Bartosz Szatkowski, linux-bluetooth On Wed, Nov 16, 2011 at 12:12 PM, Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > Hi Bartosz, > > On Wed, Nov 16, 2011 at 10:24 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote: >> >> Come on did You even read it?? This function is OBEX level (an it >> works before in similar way - only one dir at a time) - we may don't >> like specification, but seriously? Just read the patch, i exposed on >> OBEX level stuff defined in the OBEX specification (as MAP needs >> operation like "go one level up and than down" in one step) and as it >> would be possible to just add some "if" for "../dir" in previous >> version, but it is better to fix broken things than to propagate them >> (it's obex level function not high level abstraction - deal with it). > > Yeah, lets create an API that we don't use, really? Your MAP API need > it but that doesn't mean we agree on using it as it is, that table you > use to explain how it works is already a sign that it is I guess the table I've seen in Bartek's proposal is rather graphical representation that serves an example. I think that simply saying that "cdup" goes up one level before going to child directory would be also understandable and not really confusing. > over-engineered and will be easily misinterpreted by applications. We > will not break MAP by not exposing this complexity since we can still > do it transparently, even doing cdup and another SetPath although > inefficient would still work. The function itself is quite useful for MAP clients. There is this fixed part of directories structure in MAP, which is: telecom '- msg |- inbox |- outbox |- sent |- deleted '- draft There are no messages in msg, thus going from e.g. outbox to sent directly seems convenient. I do not understand why you are so reluctant to have function for setting path in gobex that actually offers what OBEX offers. As I mentioned already in discussion about MAP API proposal, this also gives additional restrictions on what characters one can use in folder name. You at least have one example of profile that could have make use of it. So it won't be in effect anything that is not used. >> Each profile may use it multiple times for paths delivered through >> dbus (but it provokes some questions, that i don't like). Generally >> there is REALLY many gui that works like that (one level down or up at >> a time) just look around (eg all file explorers) and it's all you >> need, because its basically same context! > > Same context? What context, the folder listing is per folder, besides > nowadays the file explores have abandon the tree view in favor of more > simple one level like nautilus. But even with gui that uses tree view > we can only jump one level or to the root so it is not that it would > save that much of time. -- Slawomir Bochenski ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] gobex: Fix setpath to match one from OBEX spec 2011-11-16 11:49 ` Slawomir Bochenski @ 2011-11-16 12:00 ` Johan Hedberg 0 siblings, 0 replies; 6+ messages in thread From: Johan Hedberg @ 2011-11-16 12:00 UTC (permalink / raw) To: Slawomir Bochenski Cc: Luiz Augusto von Dentz, Bartosz Szatkowski, linux-bluetooth Hi, Slawek, On Wed, Nov 16, 2011, Slawomir Bochenski wrote: > The function itself is quite useful for MAP clients. There is this > fixed part of directories structure in MAP, which is: > > telecom > '- msg > |- inbox > |- outbox > |- sent > |- deleted > '- draft > > There are no messages in msg, thus going from e.g. outbox to sent > directly seems convenient. > > I do not understand why you are so reluctant to have function for > setting path in gobex that actually offers what OBEX offers. As I > mentioned already in discussion about MAP API proposal, this also > gives additional restrictions on what characters one can use in folder > name. > > You at least have one example of profile that could have make use of > it. So it won't be in effect anything that is not used. I don't as such have anything against the possibility of supporting a "cd ../foo" operation through the API, but the way that the OBEX spec implements it feels like a hack that's been put in place as an afterthought (they thought they'd get away with restricting path changes to one level at a time and when they realized ../foo might make sense instead of extending the name header they added this to flags (I don't know if this is how things actually happened but it's the impression I get). In OBEX 1.4 the DestName header was defined to tackle this shortcoming of the Name header, but at that point it was too late to have SetPath use it due to backwards compatibility reasons. Anyway, there's no reason to expose this ugliness in our APIs and we can still support the feature by allowing the application to pass "../foo" as the path name. Johan ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-11-16 12:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-15 14:44 [PATCH] gobex: Fix setpath to match one from OBEX spec Bartosz Szatkowski 2011-11-15 22:29 ` Luiz Augusto von Dentz 2011-11-16 8:24 ` Bartosz Szatkowski 2011-11-16 11:12 ` Luiz Augusto von Dentz 2011-11-16 11:49 ` Slawomir Bochenski 2011-11-16 12:00 ` Johan Hedberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).