Passing an empty string as a filename for obc_transfer_get will be
similar to passing a NULL filename. This means a temporary file will be
created to store the content of the transfer.
NULL and "" are not exactly equivalent though: in case of NULL the file
will be automatically removed immediately after being open, which means
that the transfer initiator should also open the file to prevent it from
being removed (to be used from the modules). In this case, the filename
will not be exposed in D-Bus.
On the other hand, if "" is given, the file will be removed only in case
of error. So after success the transfer initiator should decide whether
the file should be removed or not.
This change is convenient in order to expose the same API in D-Bus.
There is no reason to have inconsistent behavior between GetFile and
PutFile, in FileTransfer D-Bus API.
Before this change, PutFile reported success immediately after queueing
the transfer, even though the D-Bus signature includes the async flag.
Relying on a internal policy (based on transfer type) to decide if a
transfer should be exposed or not in D-Bus has some limitations. The
simplest possible alternative to this is to expose all transfers in
D-Bus, assuming the overhead is not significant.
The authorization mechanism is entirely removed from the session, and
thus transfers are automatically started (once popped from the queue)
without confirmation and without any name/filename change.
The number of transferred bytes is exposed in D-Bus using a specific
property for this purpose.
Internally, the value of this property does not necessarily match the
internal progress counter. In order to avoid D-Bus overhead, the
property will be updated once per second.
After this patch the modules are responsible for creating the transfers,
and these objects must be queued using the session API.
This way the transfer initiator has full access to the transfer object,
in case for example it wants to access some member variable.
Transfer API now takes const buffers (both params and contents) and
internally copies the memory as necessary. This new API is safer to use,
which is convenient if the modules would start using it directly.
The transfer-creating functions (obc_transfer_get and obc_transfer_put)
no longer register the transfer automatically.
This separation makes it possible that the modules would create the
transfers and then pass the object to the session, which would be
responsible for the registration.
The creation process has been internally split into two steps: creation
and D-Bus registration. This is easier to understand and it also allows
to expose these two-steps in the transfer API.
obc_transfer_get() and obc_transfer_put() should only assume ownership
of the given params only in case of success. Otherwise some erros might
result in a double free of such memory.
It is safer to remove the transfer from the internal queue (including
session->p) before calling the transfer callback. This makes sure the
callback will not manipulate the session in a way that the transfer is
removed more than once.
This was previously protected with session->p->id != 0 checks, but once
the new callbacks have been adopted in session API, this logic can be
removed.
Operations involving a transfer object will receive a pointer to such
transfer in the callback.
Note that the ownership of this object is not changed in any way,
meaning that the session is still responsible for it. However this
pointer can be useful during the execution of the callback, in order to
access data members of the transfer.
Trivial changes in buffer getters in both session and transfer,
regarding the access of transfer parameters:
- const qualifiers added, to avoid unwanted frees
- Buffers are now returned as void* instead of guint8*
This simplify the API a bit by not having to call obc_transfer_set_file
to open the file.
In addition to that split transfer creation/registration function so
GET/PUT can have more specific logic and different paramenters.
A new enum type is used to distinguish put and get transfers.
This is more convenient since it is done when registering the transfer,
and not when it is actually started. The main benefits would be:
- Some actions can be taken during creation, such as opening files.
- session.c gets simplified.
- The size of a put transfer can be exposed in D-Bus, while queued.
- The transfer operation (put or get) can be exposed in D-Bus.
None of these D-Bus changes are included in this patch.
The errorcode field is set but never used, so it can safely be removed.
In addition there is no need for such a field, because errors can be
propagated using the available callback.
16 bytes in 1 blocks are definitely lost in loss record 26 of 146
at 0x4A075B2: realloc (vg_replace_malloc.c:525)
by 0x3B5104B76D: g_realloc (in /lib64/libglib-2.0.so.0.3000.2)
by 0x3B51064A96: ??? (in /lib64/libglib-2.0.so.0.3000.2)
by 0x3B51065156: g_string_insert_len (in /lib64/libglib-2.0.so.0.3000.2)
by 0x3B510305BC: ??? (in /lib64/libglib-2.0.so.0.3000.2)
by 0x3B51031BE7: g_build_filename (in /lib64/libglib-2.0.so.0.3000.2)
by 0x416FEE: pbap_select (pbap.c:254)
by 0x406CCE: process_message (object.c:224)
by 0x3B5301D9A0: ??? (in /lib64/libdbus-1.so.3.5.6)
by 0x3B5300F92F: dbus_connection_dispatch (in /lib64/libdbus-1.so.3.5.6)
Commit c07ddfbd019d3545cce2d7ec694143cdc55a2167 introduced the freeing of
the active pending request on obc_session_shutdown without checking if
the request was already processed/terminated.
Functions obc_session_get and obc_session_pull nearly share the same
code, so the later can be achieved by just calling the first one.
The session api is not modified in this patch.
The terms "name", "filename" and "targetname" are used in session.h that
can be confusing. This patch proposes to follow the terminology in the
D-Bus api:
- Name: the remote name of the object being transferred
- Filename: the local filesystem name of a file being sent
- Targetfile: the local filesystem name of a file being
received
The terms can be quite misleading, so this patch proposes to follow the
terminology in the D-Bus api:
- Name: the remote name of the object being transferred
- Filename: the name of the file in local the filesystem
Both values can be NULL independently.
This fixes the problem of using the terms differently in get and put
operations. The result was that the properties "Name" and "Filename" were
swapped in D-Bus in the case of get.
Once the fields map to obex fields, the interpretation of the response
from the agent becomes more complicated. Depending on the transfer type,
either the name or the filename field must be updated.
Previous implementation of session_terminate_transfer assumed that the
transfer being terminated would always be the active one. However, it
should be possible to cancel any queued transfer using the D-Bus api.
The authorization request of a queued transfer could fail, and this
needs to be reported to the transfer initiator. Otherwise it would
likely result in D-Bus timeouts.
There is no much of point to have a user_data if it is always the same
type, besides this code is very inefficient and cause a lookup in the list
of pending calls everytime a reply is received.
This fixes regression introduced by
63becff48820dc50a30ae495e286e858a886d9dd, causing obex-client to crash
in cases of e.g. remote site rejecting pushed file.
The req->function set by user of agent API may request agent object
deletion. This in turn checks if agent->pending is set and if it is,
it tries to cancel the pending call and frees pending call data. As at
this point we are already handling call response and we are going to
free this pending call data, agent->pending can be set to NULL prior to
calling req->function, thus preventing premature freeing of later
dereferenced req.
Invalid read of size 8
at 0x413DA1: get_file_callback (ftp.c:184)
by 0x40A74E: transfer_complete (gobex-transfer.c:73)
by 0x40AB91: transfer_response (gobex-transfer.c:172)
by 0x40847A: handle_response (gobex.c:629)
by 0x408C06: incoming_data (gobex.c:811)
by 0x3E01043DBC: g_main_context_dispatch (in /lib64/libglib-2.0.so.0.2910.0)
by 0x3E010445A7: ??? (in /lib64/libglib-2.0.so.0.2910.0)
by 0x3E01044AF4: g_main_loop_run (in /lib64/libglib-2.0.so.0.2910.0)
by 0x404CD4: main (main.c:102)
Address 0x8 is not stack'd, malloc'd or (recently) free'd
This simplify target matching to a single place making it easier to add
new targets/profiles.
Matching is done by either friendly name e.g. opp, ftp... or Bluetooth
UUID.
Drivers are probed when a session is established and removed when the
session is destroyed.
Session data should not be acessible directly otherwise it cause too
much dependency by profile specific code which is quite inefficient in
the long term.
Connection of RFCOMM and SDP are extracted from session_create function
into session_connect. Such allows making asynchronous calls before
creating connections.
From stat documentation:
"(stat()) path refers to a file whose size cannot be represented in the
type off_t. This can occur when an application compiled on a 32-bit
platform without -D_FILE_OFFSET_BITS=64 calls stat() on a file whose size
exceeds (2<<31)-1 bits."
To fix this now size header is omitted when the file is over 32-bit, but
it is able to transfer it by using 64-bit variables. In addition to that
folder-listing now should report such big sizes properly.
Since obex-client and obexd share the same log code they both were using
obexd for openlog which makes it very confusing when reading the logs.
To fix this now __obex_log_init takes the binary name so that each daemon
can be properly labeled.
If the callback removes the pending data it cause this:
==20639== Invalid read of size 4
==20639== at 0x80553E9: free_pending (session.c:112)
==20639== by 0x8056C83: session_request_reply (session.c:837)
==20639== by 0x412F7E0: ??? (in /lib/libdbus-1.so.3.5.2)
==20639== by 0x411D975: ??? (in /lib/libdbus-1.so.3.5.2)
==20639== by 0x4120B81: dbus_connection_dispatch (in /lib/libdbus-1.so.3.5.2)
==20639== by 0x804C27F: message_dispatch (mainloop.c:80)
==20639== by 0x407EFCB: ??? (in /lib/libglib-2.0.so.0.2600.1)
==20639== by 0x407E854: g_main_context_dispatch (in /lib/libglib-2.0.so.0.2600.1)
==20639== by 0x4082667: ??? (in /lib/libglib-2.0.so.0.2600.1)
==20639== by 0x4082BA6: g_main_loop_run (in /lib/libglib-2.0.so.0.2600.1)
==20639== by 0x8055171: main (main.c:625)
==20639== Address 0x4363c88 is 0 bytes inside a block of size 12 free'd
==20639== at 0x40257ED: free (vg_replace_malloc.c:366)
==20639== by 0x4087485: g_free (in /lib/libglib-2.0.so.0.2600.1)
==20639== by 0x80553FE: free_pending (session.c:115)
==20639== by 0x805543C: agent_free (session.c:127)
==20639== by 0x80566A6: session_free (session.c:149)
==20639== by 0x8056BCA: session_terminate_transfer (session.c:914)
==20639== by 0x8056F61: session_prepare_put (session.c:1397)
==20639== by 0x8056C74: session_request_reply (session.c:835)
==20639== by 0x412F7E0: ??? (in /lib/libdbus-1.so.3.5.2)
==20639== by 0x411D975: ??? (in /lib/libdbus-1.so.3.5.2)
==20639== by 0x4120B81: dbus_connection_dispatch (in /lib/libdbus-1.so.3.5.2)
==20639== by 0x804C27F: message_dispatch (mainloop.c:80)
To fix this agent->pending is now reset to NULL before calling the
callback, so even if the session is terminated it won't cause a free to
pending data, which is fine since it is latter freed on callback return.
If the call to the Request method of the obex client agent returns
with an error (for example if the transfer is rejected), call
function session_terminate_transfer instead of unregister_transfer
to both unregister the transfer and handle the pending request.
Make use of gw_obex_xfer_close instead of gw_obex_xfer_flush since the
former not only flushes the remaining data but also wait for the response
catching errors that gw_obex_xfer_flush doesn't.
When connection attempt fails the socket were left opened as it is not
assigned to the session, also when the connection does succeed the socket
is closed twice when the session is removed.
To fix those issues session now holds a reference to the GIOChannel
returned bt bt_io_connect so that the connection can properly close when
releasing, in addiction to that it also is marked to not close the socket
when the connection succeeds so that when removing the session it doesn't
close the socket twice.
Thanks for Vitja Makarov <vitja.makarov@gmail.com> for reporting this.
This should also improve speed since now file transfer also tries to read
all buffered data before continue, so each progress will probably be
around the MTU size rather than buffer size.
This reply was being sent too early and it was being sent on the complete
callback anyway. This was causing the sender to exit the bus too soon.
Which was causing the daemon to close the connection.
This requession was introduced by d57bffe46b71e17a640c11b389dd6da95f933729
that add another reference to the session for the agent.
To fix this a rework on refcount was done so that transfer now hold
references to the session and once done they release the references one
by one.
The response was being ignored, now this will allow the agent to cancel
operations it doesn't want anymore and to change the name of the
object being sent.
These were represented as ssize_t which represent the maximum size
of the addressable memory, so they are 32bits in 32bit machines. We
were expecting them to be 64bits.
As there are some special conditions involving a folder listing as
no object size, storing everything inside a buffer, this is better left
apart from the "normal" case.
