On 04/05/2010 10:56 AM, Daniel Stenberg wrote:
> On Fri, 2 Apr 2010, Ben Greear wrote:
>
>> Ok, I have another try at imap here. It can download messages that do
>> not fit into a single pingpong cache read now. I also removed some
>> cruft that was assuming pingpoing headers could contain more than one
>> line of info.
>
>> This patch also exports the transfer.c Transfer() method as
>> Curl_do_transfer because imap needed to use it.
>
> That is not a good idea. It proves it's not done right and that it won't
> be multi-interface friendly enough. Why does imap need to call it?

I am sure I'm not doing it right..but I'm having a hell of a time
figuring out the right way to do it.

Here is one scenario: You request 3 email, which comes to around 1500 bytes
total. The first two, and most of the third end up in the cache in that
callback, but an extra 60 or so bytes must be downloaded.

My hack of doing the transfer works, but seems like some part of pingpong
or some other logic would do that Transfer logic.

What logic *should* call that Transfer in this case?

>> One un-fixed problem is that I don't know how to deal with reading
>> headers (say, the results of a search) that do not fit in the pingpong
>> cache. I don't know the full size, so I can't use the Transfer logic,
>> and the pingpong stuff doesn't seem to like headers that don't
>> completey fit in cache.
>
> No it doesn't. It treats headers as... headers and they are in fact
> never very long (in a sane environment) and if they are very long,
> libcurl will deliberately chop them up to survive and deal with them
> somehow.

Ok, what if I get the results of a search, and it's something like:

* SEARCH [25k of data]
D OK Search Completed

As far as I can tell, there is no limit on the amount of data after
the SEARCH string.

How is pingpong going to deal with this?

If not all of that SEARCH data is in pp->cache, how do I properly download the
rest of it without knowing the exact number of bytes that I need to read?

> A few words of advice:
>
> - don't include code #if 0'ed
> - don't use printf(), use infof() or DEBUGF() - curl style!

I know. This is for my own debugging use..will remove before I
post anything for inclusion in curl.

> - there seems to be repeated code handling the pp->cache in numerous places
> which seems it can use some refactoring to reduce code duplication

Yes.

> - I suggest you start making sure these features can be tested using
> the curl test suite and then add tests for all these new features. Parts of
> this code gets a bit tricky to follow and we need tests to make it possible
> for us to understand better.
> - remember the multi interface, we need to be able to use that just as well
> as the easy interface so one or two test cases that use that interface is
> a good idea to verify functionality
>
> I much rather see you working closer to what we want, as then we can
> soon merge your first changes step by step into mainline instead of
> seeing you work more and more away on your own and then one day try to
> hand over the biggest patch in the universe in one go!

Well, it's good you didn't include my earlier patches that fixed a few
small problems, because that was proved broken when I tried more
advanced stuff like downloading multiple messages. Likely my current
patch is also full of bugs (and not just the SEARCH thing which I'm
almost certain won't work right in all cases now).

When I can get the Transfer logic and general interaction with
pingpong working more to your liking, I'll work on curl test
cases.

Thanks,
Ben

-- 
Ben Greear <greearb_at_candelatech.com>
Candela Technologies Inc  http://www.candelatech.com
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html