Hi Christian, I'm sending this to the list again, I guess you forgot that in your message? Or was it intentional?
> thanks for all your work and the thoughtful mail! I never bothered to
> come up with smart caching. Maybe with your ideas and help this could
> now be improved in ObexFTP ;)
I've taken the cache improvements a bit further, and improved the cache
handling in obexftp and changed obexfs to support rename properly.
You can find the changes at:
http://git.stderr.nl/matthijs/upstream/obexftp.git master
http://git.stderr.nl/matthijs/upstream/obexfs.git master
For easy reviewing, I'll also send the patches to the list (two series, one
for obexftp, one for obexfs), as replies to this message.
The ObexFS commits are mostly trivial, the ObexFTP commits are more
interesting and involved. Most of this email is about the ObexFTP
changes.
Most of the commits should be small and easy to review (but do read the
extended commit logs, not just the first line). I still have some
doubts, questions, or an urge to further explain some commits, which I
will do so below. Any commit that causes a change in the obexftp API
should also be listed below.
All commits have been tested individually, though I have been optimizing
some changes afterwards for easier reviewing (reordering and merging
commits). These changes were compile tested, but not completely
retested. I did, however, completely review all changes again, so I am
confident that the code is mostly good.
Here come the commits and comments, in no particular order.
1d5ee73c9398916604ec00738649ab405f3c1381
Always pass normalized paths to obexftp_cache_list.
This commit changes control flow a bit, resulting in less normalization.
However, it also means that the call
cli->infocb(OBEXFTP_EV_RECEIVING, path, 0, cli->infocb_data);
now gets a normalized path, instead of the original path passed to
obexftp_opendir and obexftp_stat. I'm not sure how this infocb stuff works,
but is this a problem? I guess it would at least constitute an API change.
951ba12157f8b3b7d27df5b9c5b0cd91c16cd362
Restructure obexftp_cache_list.
This commit changes the handling of the /telecom builtin stuff, though
I'm not sure what the purpose of that is. The old code used to insert a
dummy entry containing just "devinfo.txt" if the path requested was
/telecom/. Then, the device would still be asked for a listing, which
(due to the ordering of the stack) would be returned instead of the hard
coded listing (if the device returned any listing, of course).
I've made this "fallback" a bit more explicit, but I'm not completely
sure this is the intended behaviour?
3d98fbd72b6136662b650ebb315ee2cd52dd695d
Allow obexftp_setpath to create only the last directory.
This commit extends the obexftp_setpath function to support
non-recursive directory creation. This is done by extending the "create"
parameter to support 2 for non-recursive creation. This is an extension
to the API, that should be compatible for clients that use 0 and 1 as
values for create. Clients that use 0 and non-zero will break, since
anything but 1 and 2 is treated as 0 (no creation).
73672c76ba0cc1ba414ab8ea4aa66fcc314e461e
Rename cache_object_t.stats to entries.
a6ec78cd7fef337dcf20dbdd544c2c492f8627b8
Make get_cache_object return the cache_object_t.
a7984d0ec59e65f6b5c5dd82d851a18fd6720e39
Make put_cache_object return the new cache_object_t.
56100a3b20e94d03f8d0d522ff1164692ea97702
Always normalize names used in the cache the same way.
2f5357a71f6999fa082262abf8084a32512451ae
Make the cache_purge always empty the cache.
1cbd03229eecb302adb71cd3a84b6c36c8ea481c
Make cache_purge accept a obexft_client_t*.
fadb63bbd8ad73a369cb97e86a8fa51064701a2a
Make the cached entries list a linked list.
84a56a9d610743fc42fce3696ff7be9186d82d22
Always fill cache->entries immediately.
These commits change the internal structure of the cache. Since these
functions and structures are exposed through cache.h, I guess these are also
API changes. However, it's probably better to make these static, or at least
document they are internal.
4446447a5f93f4eceac0b1d9bcc8a6dab00697e5
Add a cache_update function and use it in client.c.
This commit extends the API (if cache.[ch] is part of the API) with a
new function cache_update.
c9e1678f9df1334d135ce2f4660348b3322d3dd1
Report iconv errors only when they occur.
857da02539a3b79b2c249b42e11a52c4ac302ad5
On an iconv error, show the proper error string.
6f9b65875d0d7a831af3b5a8a3310042d76af3db
Let Utf8ToChar return an error code on iconv errors.
5efdfaf99fa07f570bc8cce768a33fc346c6ff2d
01e59c8dc43e3f91671e1f17f94f2106d4971339
Let Utf8ToChar also convert the trailing \0.
The first three commits improve the error handling of Utf8ToChar, which
was wrong and confused me while debugging an iconv issue. The last
commit fixes this issue, which could cause out of bound reads in some
cases (seems to work most of the time, probably due to the heavy use of
calloc).
A separate concern, which is not fixed in any commit, is that of memory
allocation. It seems that at least obexfs fails to properly free the
cli->data structure, resulting in lots of "Warning: buffer still
active?" messages. I think the main reason for this is the fact that
the data is returned in the cli->data member, invisible from the
function signature. I would propose to instead make the functions that
perform requests to explicitely return the data, using a pointer
argument (e.g, char** or something). However, since this is a rather
invasive (API) change, I haven't implemented this yet. What you do think
of this?
I hope you can find the time to review these patches and provide
feedback or commit them. If you have any other wishes regarding the
format, style, ordering, or whatever of the patches, just let me know
and I'll have a look.
Gr.
Matthijs
signature.asc
Description: Digital signature
------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________ Openobex-users mailing list Openobex-users@lists.sourceforge.net http://lists.sourceforge.net/lists/listinfo/openobex-users
