Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c32287f8-1604-1571-703e-10ed8ccbc192@gmail.com>
Date:   Thu, 20 Apr 2023 17:02:58 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.10.0
Subject: Re: [PATCH BlueZ v4] obexd: support to reply folder name to store
 file
Content-Language: en-US
To:     Paul Menzel <pmenzel@molgen.mpg.de>
Cc:     linux-bluetooth@vger.kernel.org
References: <20230420022554.14159-1-aarongt.shen@gmail.com>
 <c78e25ba-9cfa-9032-0cd8-3dc6c265558c@molgen.mpg.de>
From:   Guiting Shen <aarongt.shen@gmail.com>
In-Reply-To: <c78e25ba-9cfa-9032-0cd8-3dc6c265558c@molgen.mpg.de>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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