* [PATCH obexd] Fix file size issue in GetProperties
@ 2011-12-02 11:57 jaganath.k
2011-12-02 12:41 ` Luiz Augusto von Dentz
2011-12-07 15:19 ` Johan Hedberg
0 siblings, 2 replies; 6+ messages in thread
From: jaganath.k @ 2011-12-02 11:57 UTC (permalink / raw)
To: linux-bluetooth; +Cc: jaganath.k
From: jaganath.k <jaganath.k@samsung.com>
In SendFiles, If application calls GetProperties before
Request method returns then the file size is zero.
---
client/session.c | 4 ++++
client/transfer.c | 40 ++++++++++++++++++++++++----------------
client/transfer.h | 1 +
3 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/client/session.c b/client/session.c
index f288ecd..9f02512 100644
--- a/client/session.c
+++ b/client/session.c
@@ -1277,6 +1277,10 @@ int obc_session_send(struct obc_session *session, const char *filename,
if (transfer == NULL)
return -EINVAL;
+ err = obc_transfer_set_file(transfer);
+ if (err < 0)
+ goto fail;
+
/* Transfer should start if it is the first in the pending list */
if (transfer != session->pending->data)
return 0;
diff --git a/client/transfer.c b/client/transfer.c
index b6994d1..5b67d1e 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -563,8 +563,6 @@ int obc_transfer_put(struct obc_transfer *transfer, transfer_callback_t func,
GObex *obex;
GObexPacket *req;
GObexDataProducer data_cb;
- struct stat st;
- int fd;
if (transfer->xfer != 0)
return -EALREADY;
@@ -574,20 +572,6 @@ int obc_transfer_put(struct obc_transfer *transfer, transfer_callback_t func,
goto done;
}
- fd = open(transfer->filename, O_RDONLY);
- if (fd < 0) {
- error("open(): %s(%d)", strerror(errno), errno);
- return -errno;
- }
-
- if (fstat(fd, &st) < 0) {
- close(fd);
- error("fstat(): %s(%d)", strerror(errno), errno);
- return -errno;
- }
-
- transfer->fd = fd;
- transfer->size = st.st_size;
data_cb = put_xfer_progress;
done:
@@ -667,3 +651,27 @@ gint64 obc_transfer_get_size(struct obc_transfer *transfer)
{
return transfer->size;
}
+
+int obc_transfer_set_file(struct obc_transfer *transfer)
+{
+ int fd;
+ struct stat st;
+
+ fd = open(transfer->filename, O_RDONLY);
+ if (fd < 0) {
+ error("open(): %s(%d)", strerror(errno), errno);
+ return -errno;
+ }
+
+ if (fstat(fd, &st) < 0) {
+ error("fstat(): %s(%d)", strerror(errno), errno);
+ close(fd);
+ return -errno;
+ }
+
+ transfer->fd = fd;
+ transfer->size = st.st_size;
+
+ return 0;
+}
+
diff --git a/client/transfer.h b/client/transfer.h
index ba53fd1..c051cbd 100644
--- a/client/transfer.h
+++ b/client/transfer.h
@@ -55,3 +55,4 @@ void obc_transfer_clear_buffer(struct obc_transfer *transfer);
void obc_transfer_set_name(struct obc_transfer *transfer, const char *name);
const char *obc_transfer_get_path(struct obc_transfer *transfer);
gint64 obc_transfer_get_size(struct obc_transfer *transfer);
+int obc_transfer_set_file(struct obc_transfer *transfer);
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH obexd] Fix file size issue in GetProperties
2011-12-02 11:57 [PATCH obexd] Fix file size issue in GetProperties jaganath.k
@ 2011-12-02 12:41 ` Luiz Augusto von Dentz
2011-12-02 13:00 ` Jaganath
2011-12-07 15:19 ` Johan Hedberg
1 sibling, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2011-12-02 12:41 UTC (permalink / raw)
To: jaganath.k; +Cc: linux-bluetooth
Hi Jaganath,
On Fri, Dec 2, 2011 at 1:57 PM, <jaganath.k@samsung.com> wrote:
> From: jaganath.k <jaganath.k@samsung.com>
>
> In SendFiles, If application calls GetProperties before
> Request method returns then the file size is zero.
> ---
> client/session.c | 4 ++++
> client/transfer.c | 40 ++++++++++++++++++++++++----------------
> client/transfer.h | 1 +
> 3 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/client/session.c b/client/session.c
> index f288ecd..9f02512 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -1277,6 +1277,10 @@ int obc_session_send(struct obc_session *session, const char *filename,
> if (transfer == NULL)
> return -EINVAL;
>
> + err = obc_transfer_set_file(transfer);
> + if (err < 0)
> + goto fail;
> +
> /* Transfer should start if it is the first in the pending list */
> if (transfer != session->pending->data)
> return 0;
> diff --git a/client/transfer.c b/client/transfer.c
> index b6994d1..5b67d1e 100644
> --- a/client/transfer.c
> +++ b/client/transfer.c
> @@ -563,8 +563,6 @@ int obc_transfer_put(struct obc_transfer *transfer, transfer_callback_t func,
> GObex *obex;
> GObexPacket *req;
> GObexDataProducer data_cb;
> - struct stat st;
> - int fd;
>
> if (transfer->xfer != 0)
> return -EALREADY;
> @@ -574,20 +572,6 @@ int obc_transfer_put(struct obc_transfer *transfer, transfer_callback_t func,
> goto done;
> }
>
> - fd = open(transfer->filename, O_RDONLY);
> - if (fd < 0) {
> - error("open(): %s(%d)", strerror(errno), errno);
> - return -errno;
> - }
> -
> - if (fstat(fd, &st) < 0) {
> - close(fd);
> - error("fstat(): %s(%d)", strerror(errno), errno);
> - return -errno;
> - }
> -
> - transfer->fd = fd;
> - transfer->size = st.st_size;
> data_cb = put_xfer_progress;
>
> done:
> @@ -667,3 +651,27 @@ gint64 obc_transfer_get_size(struct obc_transfer *transfer)
> {
> return transfer->size;
> }
> +
> +int obc_transfer_set_file(struct obc_transfer *transfer)
> +{
> + int fd;
> + struct stat st;
> +
> + fd = open(transfer->filename, O_RDONLY);
> + if (fd < 0) {
> + error("open(): %s(%d)", strerror(errno), errno);
> + return -errno;
> + }
> +
> + if (fstat(fd, &st) < 0) {
> + error("fstat(): %s(%d)", strerror(errno), errno);
> + close(fd);
> + return -errno;
> + }
> +
> + transfer->fd = fd;
> + transfer->size = st.st_size;
> +
> + return 0;
> +}
> +
> diff --git a/client/transfer.h b/client/transfer.h
> index ba53fd1..c051cbd 100644
> --- a/client/transfer.h
> +++ b/client/transfer.h
> @@ -55,3 +55,4 @@ void obc_transfer_clear_buffer(struct obc_transfer *transfer);
> void obc_transfer_set_name(struct obc_transfer *transfer, const char *name);
> const char *obc_transfer_get_path(struct obc_transfer *transfer);
> gint64 obc_transfer_get_size(struct obc_transfer *transfer);
> +int obc_transfer_set_file(struct obc_transfer *transfer);
> --
Nice catch, but I would like to avoid another public function for
this, so how about making obc_transfer_set_file static on transfer.c
calling it in obc_transfer_register?
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH obexd] Fix file size issue in GetProperties
2011-12-02 12:41 ` Luiz Augusto von Dentz
@ 2011-12-02 13:00 ` Jaganath
2011-12-02 14:31 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 6+ messages in thread
From: Jaganath @ 2011-12-02 13:00 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
--------------------------------------------------
From: "Luiz Augusto von Dentz" <luiz.dentz@gmail.com>
Sent: Friday, December 02, 2011 6:11 PM
To: <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH obexd] Fix file size issue in GetProperties
> Hi Jaganath,
>
> On Fri, Dec 2, 2011 at 1:57 PM, <jaganath.k@samsung.com> wrote:
>> From: jaganath.k <jaganath.k@samsung.com>
>>
>> In SendFiles, If application calls GetProperties before
>> Request method returns then the file size is zero.
>> ---
>> client/session.c | 4 ++++
>> client/transfer.c | 40 ++++++++++++++++++++++++----------------
>> client/transfer.h | 1 +
>> 3 files changed, 29 insertions(+), 16 deletions(-)
>>
>> diff --git a/client/session.c b/client/session.c
>> index f288ecd..9f02512 100644
>> --- a/client/session.c
>> +++ b/client/session.c
>> @@ -1277,6 +1277,10 @@ int obc_session_send(struct obc_session *session,
>> const char *filename,
>> if (transfer == NULL)
>> return -EINVAL;
>>
>> + err = obc_transfer_set_file(transfer);
>> + if (err < 0)
>> + goto fail;
>> +
>> /* Transfer should start if it is the first in the pending list */
>> if (transfer != session->pending->data)
>> return 0;
>> diff --git a/client/transfer.c b/client/transfer.c
>> index b6994d1..5b67d1e 100644
>> --- a/client/transfer.c
>> +++ b/client/transfer.c
>> @@ -563,8 +563,6 @@ int obc_transfer_put(struct obc_transfer *transfer,
>> transfer_callback_t func,
>> GObex *obex;
>> GObexPacket *req;
>> GObexDataProducer data_cb;
>> - struct stat st;
>> - int fd;
>>
>> if (transfer->xfer != 0)
>> return -EALREADY;
>> @@ -574,20 +572,6 @@ int obc_transfer_put(struct obc_transfer *transfer,
>> transfer_callback_t func,
>> goto done;
>> }
>>
>> - fd = open(transfer->filename, O_RDONLY);
>> - if (fd < 0) {
>> - error("open(): %s(%d)", strerror(errno), errno);
>> - return -errno;
>> - }
>> -
>> - if (fstat(fd, &st) < 0) {
>> - close(fd);
>> - error("fstat(): %s(%d)", strerror(errno), errno);
>> - return -errno;
>> - }
>> -
>> - transfer->fd = fd;
>> - transfer->size = st.st_size;
>> data_cb = put_xfer_progress;
>>
>> done:
>> @@ -667,3 +651,27 @@ gint64 obc_transfer_get_size(struct obc_transfer
>> *transfer)
>> {
>> return transfer->size;
>> }
>> +
>> +int obc_transfer_set_file(struct obc_transfer *transfer)
>> +{
>> + int fd;
>> + struct stat st;
>> +
>> + fd = open(transfer->filename, O_RDONLY);
>> + if (fd < 0) {
>> + error("open(): %s(%d)", strerror(errno), errno);
>> + return -errno;
>> + }
>> +
>> + if (fstat(fd, &st) < 0) {
>> + error("fstat(): %s(%d)", strerror(errno), errno);
>> + close(fd);
>> + return -errno;
>> + }
>> +
>> + transfer->fd = fd;
>> + transfer->size = st.st_size;
>> +
>> + return 0;
>> +}
>> +
>> diff --git a/client/transfer.h b/client/transfer.h
>> index ba53fd1..c051cbd 100644
>> --- a/client/transfer.h
>> +++ b/client/transfer.h
>> @@ -55,3 +55,4 @@ void obc_transfer_clear_buffer(struct obc_transfer
>> *transfer);
>> void obc_transfer_set_name(struct obc_transfer *transfer, const char
>> *name);
>> const char *obc_transfer_get_path(struct obc_transfer *transfer);
>> gint64 obc_transfer_get_size(struct obc_transfer *transfer);
>> +int obc_transfer_set_file(struct obc_transfer *transfer);
>> --
>
> Nice catch, but I would like to avoid another public function for
> this, so how about making obc_transfer_set_file static on transfer.c
> calling it in obc_transfer_register?
>
> --
> Luiz Augusto von Dentz
> --
obc_transfer_register is being called from obc_session_get which
eventually calls obc_transfer_get in which if transfer->name is NULL
transfer->filename is open with write mode. obc_session_get is used from
pbap and sync with name as NULL and filename as NOT NULL.
Thanks and Regards
Jaganath
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH obexd] Fix file size issue in GetProperties
2011-12-02 13:00 ` Jaganath
@ 2011-12-02 14:31 ` Luiz Augusto von Dentz
2011-12-02 14:58 ` Jaganath
0 siblings, 1 reply; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2011-12-02 14:31 UTC (permalink / raw)
To: Jaganath; +Cc: linux-bluetooth
Hi Jaganath,
On Fri, Dec 2, 2011 at 3:00 PM, Jaganath <jaganath.k@samsung.com> wrote:
>>
>>
>> Nice catch, but I would like to avoid another public function for
>> this, so how about making obc_transfer_set_file static on transfer.c
>> calling it in obc_transfer_register?
>>
>> --
>> Luiz Augusto von Dentz
>> --
>
>
> obc_transfer_register is being called from obc_session_get which
> eventually calls obc_transfer_get in which if transfer->name is NULL
> transfer->filename is open with write mode. obc_session_get is used from
> pbap and sync with name as NULL and filename as NOT NULL.
Now I realize why I didn't do it since the beginning, because the
application can skip the files it is probably better to not open them
until they are acknowledged, also it is very bad idea to call back the
daemon while within the Request as the daemon could be waiting the
application to acknowledged it can cause a deadlock where both the
application and obex-client are waiting for each other.
There are plans to change this API so OPP would also register a
session and all transfers register an object, with this we will
probably not need the agent interface anymore and things became a bit
simpler as we return the transfer object directly on SendFile.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH obexd] Fix file size issue in GetProperties
2011-12-02 14:31 ` Luiz Augusto von Dentz
@ 2011-12-02 14:58 ` Jaganath
0 siblings, 0 replies; 6+ messages in thread
From: Jaganath @ 2011-12-02 14:58 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
--------------------------------------------------
From: "Luiz Augusto von Dentz" <luiz.dentz@gmail.com>
Sent: Friday, December 02, 2011 8:01 PM
To: "Jaganath" <jaganath.k@samsung.com>
Cc: <linux-bluetooth@vger.kernel.org>
Subject: Re: [PATCH obexd] Fix file size issue in GetProperties
> Hi Jaganath,
>
> On Fri, Dec 2, 2011 at 3:00 PM, Jaganath <jaganath.k@samsung.com> wrote:
>>>
>>>
>>> Nice catch, but I would like to avoid another public function for
>>> this, so how about making obc_transfer_set_file static on transfer.c
>>> calling it in obc_transfer_register?
>>>
>>> --
>>> Luiz Augusto von Dentz
>>> --
>>
>>
>> obc_transfer_register is being called from obc_session_get which
>> eventually calls obc_transfer_get in which if transfer->name is NULL
>> transfer->filename is open with write mode. obc_session_get is used from
>> pbap and sync with name as NULL and filename as NOT NULL.
>
> Now I realize why I didn't do it since the beginning, because the
> application can skip the files it is probably better to not open them
> until they are acknowledged, also it is very bad idea to call back the
> daemon while within the Request as the daemon could be waiting the
> application to acknowledged it can cause a deadlock where both the
> application and obex-client are waiting for each other.
>
> There are plans to change this API so OPP would also register a
> session and all transfers register an object, with this we will
> probably not need the agent interface anymore and things became a bit
> simpler as we return the transfer object directly on SendFile.
>
> --
> Luiz Augusto von Dentz
> --
I accept that it is bad to have a dbus round trip. But I am unclear about
the deadlock
scenario since deamon is calling "Request" method asynchronously.
Since transfer is already registered before "Request" method, application
can
call "GetProperties" at any point of time.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH obexd] Fix file size issue in GetProperties
2011-12-02 11:57 [PATCH obexd] Fix file size issue in GetProperties jaganath.k
2011-12-02 12:41 ` Luiz Augusto von Dentz
@ 2011-12-07 15:19 ` Johan Hedberg
1 sibling, 0 replies; 6+ messages in thread
From: Johan Hedberg @ 2011-12-07 15:19 UTC (permalink / raw)
To: jaganath.k; +Cc: linux-bluetooth
Hi Jaganath,
On Fri, Dec 02, 2011, jaganath.k@samsung.com wrote:
> In SendFiles, If application calls GetProperties before
> Request method returns then the file size is zero.
> ---
> client/session.c | 4 ++++
> client/transfer.c | 40 ++++++++++++++++++++++++----------------
> client/transfer.h | 1 +
> 3 files changed, 29 insertions(+), 16 deletions(-)
The patch has been applied but it generated the following warnings:
Applying: Fix file size issue in GetProperties
/home/jh/src/obexd/.git/rebase-apply/patch:83: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Please be careful with such issues in the future (and test applying the
patch yourself before applying).
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-12-07 15:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-02 11:57 [PATCH obexd] Fix file size issue in GetProperties jaganath.k
2011-12-02 12:41 ` Luiz Augusto von Dentz
2011-12-02 13:00 ` Jaganath
2011-12-02 14:31 ` Luiz Augusto von Dentz
2011-12-02 14:58 ` Jaganath
2011-12-07 15:19 ` 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).