| Message ID | 20230420022554.14159-1-aarongt.shen@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [BlueZ,v4] obexd: support to reply folder name to store file | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/BuildEll | success | Build ELL PASS |
| tedd_an/BluezMake | success | Bluez Make PASS |
| tedd_an/MakeCheck | success | Bluez Make Check PASS |
| tedd_an/MakeDistcheck | success | Make Distcheck PASS |
| tedd_an/CheckValgrind | success | Check Valgrind PASS |
| tedd_an/CheckSmatch | success | CheckSparse PASS |
| tedd_an/bluezmakeextell | success | Make External ELL PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
| tedd_an/ScanBuild | success | Scan Build PASS |
This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=741525 ---Test result--- Test Summary: CheckPatch PASS 0.43 seconds GitLint PASS 0.29 seconds BuildEll PASS 27.30 seconds BluezMake PASS 918.48 seconds MakeCheck PASS 11.54 seconds MakeDistcheck PASS 154.20 seconds CheckValgrind PASS 253.35 seconds CheckSmatch PASS 340.07 seconds bluezmakeextell PASS 101.97 seconds IncrementalBuild PASS 772.74 seconds ScanBuild PASS 1064.97 seconds --- Regards, Linux Bluetooth
Dear Guiting, Thank you for sending the fourth iteration. In the summary, I’d use also *return* instead of *reply*. Maybe: obexd: AuthorizePush: Support to return folder name for file storage Am 20.04.23 um 04:25 schrieb Guiting Shen: > The obex agent usually returns the full path by getting the default > folder and filename from the Filename property of the transfer object > which is not convenient. Just a minor thing for the future, I’d add a blank line between paragraphs. > The patch helps that the obex agent can return the folder name suffixed > with '/' or new full path or even null which will use the default name > if new_name is NULL and the default folder if the new_folder is NULL in > opp_chkput(). > --- > doc/obex-agent-api.txt | 9 +++++---- > obexd/src/manager.c | 10 ++++++---- > 2 files changed, 11 insertions(+), 8 deletions(-) > > diff --git a/doc/obex-agent-api.txt b/doc/obex-agent-api.txt > index 3923da6df..5a9c6d417 100644 > --- a/doc/obex-agent-api.txt > +++ b/doc/obex-agent-api.txt > @@ -46,10 +46,11 @@ Methods void Release() > This method gets called when the service daemon > needs to accept/reject a Bluetooth object push request. > > - Returns the full path (including the filename) where > - the object shall be stored. The tranfer object will > - contain a Filename property that contains the default > - location and name that can be returned. > + Returns the full path (including the filename) or > + folder name suffixed with '/' where the object shall > + be stored. The transfer object will contain a Filename > + property that contains the default location and name > + that can be returned. > > Possible errors: org.bluez.obex.Error.Rejected > org.bluez.obex.Error.Canceled > diff --git a/obexd/src/manager.c b/obexd/src/manager.c > index 849928603..73fd6b9af 100644 > --- a/obexd/src/manager.c > +++ b/obexd/src/manager.c > @@ -632,8 +632,7 @@ static void agent_reply(DBusPendingCall *call, void *user_data) > > if (dbus_error_has_name(&derr, DBUS_ERROR_NO_REPLY)) > agent_cancel(); > - > - if (dbus_error_has_name(&derr, OBEX_ERROR_REJECT)) > + else if (dbus_error_has_name(&derr, OBEX_ERROR_REJECT)) > agent->auth_reject = TRUE; By the way, this looks like an unrelated change? I would do such changes in separate commits. Kind regards, Paul > > dbus_error_free(&derr); > @@ -651,7 +650,10 @@ static void agent_reply(DBusPendingCall *call, void *user_data) > agent->new_name = g_strdup(name); > agent->new_folder = NULL; > } else { > - agent->new_name = g_strdup(slash + 1); > + if (strlen(slash) == 1) > + agent->new_name = NULL; > + else > + agent->new_name = g_strdup(slash + 1); > agent->new_folder = g_strndup(name, slash - name); > } > } > @@ -722,7 +724,7 @@ int manager_request_authorization(struct obex_transfer *transfer, > > dbus_pending_call_unref(call); > > - if (!agent || !agent->new_name || agent->auth_reject) > + if (!agent || agent->auth_reject) > return -EPERM; > > *new_folder = agent->new_folder;
Dear Paul, On 2023/4/20 14:57, Paul Menzel wrote: > Dear Guiting, > > > Thank you for sending the fourth iteration. In the summary, I’d use > also *return* instead of *reply*. Maybe: > > obexd: AuthorizePush: Support to return folder name for file storage Ok, I will change this in v5 patch. > > > > Am 20.04.23 um 04:25 schrieb Guiting Shen: >> The obex agent usually returns the full path by getting the default >> folder and filename from the Filename property of the transfer object >> which is not convenient. > > Just a minor thing for the future, I’d add a blank line between > paragraphs. Ok.I will change this in v5 patch. > >> The patch helps that the obex agent can return the folder name suffixed >> with '/' or new full path or even null which will use the default name >> if new_name is NULL and the default folder if the new_folder is NULL in >> opp_chkput(). >> --- >> doc/obex-agent-api.txt | 9 +++++---- >> obexd/src/manager.c | 10 ++++++---- >> 2 files changed, 11 insertions(+), 8 deletions(-) >> >> diff --git a/doc/obex-agent-api.txt b/doc/obex-agent-api.txt >> index 3923da6df..5a9c6d417 100644 >> --- a/doc/obex-agent-api.txt >> +++ b/doc/obex-agent-api.txt >> @@ -46,10 +46,11 @@ Methods void Release() >> This method gets called when the service daemon >> needs to accept/reject a Bluetooth object push request. >> - Returns the full path (including the filename) where >> - the object shall be stored. The tranfer object will >> - contain a Filename property that contains the default >> - location and name that can be returned. >> + Returns the full path (including the filename) or >> + folder name suffixed with '/' where the object shall >> + be stored. The transfer object will contain a Filename >> + property that contains the default location and name >> + that can be returned. >> Possible errors: org.bluez.obex.Error.Rejected >> org.bluez.obex.Error.Canceled >> diff --git a/obexd/src/manager.c b/obexd/src/manager.c >> index 849928603..73fd6b9af 100644 >> --- a/obexd/src/manager.c >> +++ b/obexd/src/manager.c >> @@ -632,8 +632,7 @@ static void agent_reply(DBusPendingCall *call, >> void *user_data) >> if (dbus_error_has_name(&derr, DBUS_ERROR_NO_REPLY)) >> agent_cancel(); >> - >> - if (dbus_error_has_name(&derr, OBEX_ERROR_REJECT)) >> + else if (dbus_error_has_name(&derr, OBEX_ERROR_REJECT)) >> agent->auth_reject = TRUE; > > By the way, this looks like an unrelated change? I would do such > changes in separate commits. Ok.I will split such changes in two commit. Thanks. > > > Kind regards, > > Paul > > >> dbus_error_free(&derr); >> @@ -651,7 +650,10 @@ static void agent_reply(DBusPendingCall *call, >> void *user_data) >> agent->new_name = g_strdup(name); >> agent->new_folder = NULL; >> } else { >> - agent->new_name = g_strdup(slash + 1); >> + if (strlen(slash) == 1) >> + agent->new_name = NULL; >> + else >> + agent->new_name = g_strdup(slash + 1); >> agent->new_folder = g_strndup(name, slash - name); >> } >> } >> @@ -722,7 +724,7 @@ int manager_request_authorization(struct >> obex_transfer *transfer, >> dbus_pending_call_unref(call); >> - if (!agent || !agent->new_name || agent->auth_reject) >> + if (!agent || agent->auth_reject) >> return -EPERM; >> *new_folder = agent->new_folder; Best regards, Guiting
diff --git a/doc/obex-agent-api.txt b/doc/obex-agent-api.txt index 3923da6df..5a9c6d417 100644 --- a/doc/obex-agent-api.txt +++ b/doc/obex-agent-api.txt @@ -46,10 +46,11 @@ Methods void Release() This method gets called when the service daemon needs to accept/reject a Bluetooth object push request. - Returns the full path (including the filename) where - the object shall be stored. The tranfer object will - contain a Filename property that contains the default - location and name that can be returned. + Returns the full path (including the filename) or + folder name suffixed with '/' where the object shall + be stored. The transfer object will contain a Filename + property that contains the default location and name + that can be returned. Possible errors: org.bluez.obex.Error.Rejected org.bluez.obex.Error.Canceled diff --git a/obexd/src/manager.c b/obexd/src/manager.c index 849928603..73fd6b9af 100644 --- a/obexd/src/manager.c +++ b/obexd/src/manager.c @@ -632,8 +632,7 @@ static void agent_reply(DBusPendingCall *call, void *user_data) if (dbus_error_has_name(&derr, DBUS_ERROR_NO_REPLY)) agent_cancel(); - - if (dbus_error_has_name(&derr, OBEX_ERROR_REJECT)) + else if (dbus_error_has_name(&derr, OBEX_ERROR_REJECT)) agent->auth_reject = TRUE; dbus_error_free(&derr); @@ -651,7 +650,10 @@ static void agent_reply(DBusPendingCall *call, void *user_data) agent->new_name = g_strdup(name); agent->new_folder = NULL; } else { - agent->new_name = g_strdup(slash + 1); + if (strlen(slash) == 1) + agent->new_name = NULL; + else + agent->new_name = g_strdup(slash + 1); agent->new_folder = g_strndup(name, slash - name); } } @@ -722,7 +724,7 @@ int manager_request_authorization(struct obex_transfer *transfer, dbus_pending_call_unref(call); - if (!agent || !agent->new_name || agent->auth_reject) + if (!agent || agent->auth_reject) return -EPERM; *new_folder = agent->new_folder;