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?

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.

A few words of advice:

- don't include code #if 0'ed
- don't use printf(), use infof() or DEBUGF() - curl style!
- there seems to be repeated code handling the pp->cache in numerous places
  which seems it can use some refactoring to reduce code duplication
- 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!

--

 / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette:  http://curl.haxx.se/mail/etiquette.html

Reply via email to