Re: [Evolution-hackers] Let the porting begin

2007-11-20 Thread Philip Van Hoof
On Wed, 2007-10-24 at 11:58 -0400, Jeffrey Stedfast wrote:
 I took a look at the IDLE implementation last night and felt it went
 about it the wrong way.

Yes, you are right. I think the right fix is to create a new API called
camel_tcp_stream_wait that works like this:


int bytes = camel_tcp_stream_wait (tcp_stream, stop_fd,
 store-idle_sleep, line_buffer, 1023);

I need the amount of read-read bytes, to know where the buffer stops, so
I return it. I need a stop_fd to get the select() to early-stop. I need
a timeout for in case of connection problems. I need the buffer to put
the newly read data in and I need a size to tell it how large my buffer
max is.


.. idle_thread () ..
{

  start_idle (store);
  line_buffer = malloc (1024)
  memset (line_buffer, 0, sizeof (*line_buffer));
  ...

  while (store-idle_cont)  {
char *lasts = NULL;
int bytes = camel_tcp_stream_wait (tcp_stream, stop_fd, 
store-idle_sleep, line_buffer, 1023);

if (bytes  0) {
char *resp;
line_buffer[bytes+1] = '\0';

for (resp = strtok_r (line_buffer, \n, lasts); resp; resp = 
strtok_r (NULL, \n, lasts)) {
if (resp  strlen (resp)  1  resp[0] == '*') {
if (!idle_resp)
idle_resp = idle_response_new (folder);
consume_idle_line (store, folder, resp, 
idle_resp);
lines++;
}
}
printf (IDLE: read %d bytes %s\n, bytes, line_buffer);
}

if (!store-idle_cont || (idle_resp  idle_resp-exists_happened)) {
idle_done (store, folder, idle_resp);
stopped = TRUE;
if (!store-idle_cont)
retval = idle_resp;
} 

if (store-idle_cont) {
if (idle_resp) {
process_idle_response (idle_resp);
idle_response_free (idle_resp);
idle_resp = NULL;
printf (Processed %d lines\n, lines);
lines = 0;
}
if (stopped)
idle_start (store);
}
memset (line_buffer, 0, sizeof (*line_buffer));
  }
}

That wait is implemented like this:


static ssize_t
stream_wait (CamelTcpStream *stream, int stop_fd, int timeout, char *buffer, 
size_t max_len)
{
CamelTcpStreamRaw *raw = CAMEL_TCP_STREAM_RAW (stream);
int fd = raw-sockfd;
ssize_t nread;
int fdmax;
fd_set rdset;
u_long yes = 1;
struct timeval tv;
int res;

//ioctlsocket (fd, FIONBIO, yes);
fdmax = MAX (fd, stop_fd) + 1;

FD_ZERO (rdset);
FD_SET (fd, rdset);
FD_SET (stop_fd, rdset);

tv.tv_sec = timeout;
tv.tv_usec = 0;
nread = -1;

res = select (fdmax, rdset, 0, 0, tv);
if (res  0  !FD_ISSET (stop_fd, rdset))
nread = recv (fd, buffer, max_len, 0);
else
nread = -1;

return nread;
}


Afaics It's not true that you don't need to change any API:

The camel_stream_read() will block until all n bytes are read. This is
something you don't know during IDLE (how many bytes you can expect to
receive), and on top of that you want to continue immediately after
select returns, not keep retrying until you have n bytes (to simulate a
blocking read). You also want a dedicated stop fildescriptor, not the
standard cancel_fd as then any cancel will interfere with this (while
you want strict control over this to let other threads start and stop
IDLE).

Well ... let me put it this way: I tried it from a few angles, I didn't
succeed with the normal camel_stream_read :-(

I have reworked the IDLE implementation. You can apply this patch to
Tinymail's upstream (trunk). You most likely can't apply it to upstream
Camel. (http://svn.tinymail.org/svn/tinymail/trunk)

There's still one problem: sometimes camel_imap_folder_stop_idle is not
returning late enough (the socket is still in IDLE state when it
returns).

Other than that, it usually works.

I'm trying to fix this last problem right now, and after that there will
most likely be a few days of testing before I put this in Tinymail's
repository.


 You should simply poll() on the socket descriptors (and a pipe fd used
 for telling the IDLE thread's I/O loop to finish and send DONE).

Yep


 Jeff
 
 On Mon, 2007-10-08 at 00:41 +0200, Philip Van Hoof wrote:
  On Sun, 2007-10-07 at 14:15 +0200, Philip Van Hoof wrote:
   Hi there,
  
   Using this changeset you can follow the changes to camel-lite:
   
 http://tinymail.org/trac/tinymail/changeset/2823
   
  
  This changeset are a bunch of compilation warnings for Matthew's Base64
  patch to Camel: http://tinymail.org/trac/tinymail/changeset/2827
  
  
  ps. Adding Matthew 

Re: [Evolution-hackers] Let the porting begin

2007-10-25 Thread Jeffrey Stedfast

On Fri, 2007-10-26 at 03:25 +0200, Philip Van Hoof wrote:
 On Wed, 2007-10-24 at 11:58 -0400, Jeffrey Stedfast wrote:
  I took a look at the IDLE implementation last night and felt it went
  about it the wrong way.
 
 Yes, you are right. I think the right fix is to create a new API called
 camel_tcp_stream_wait that works like this:
 
 
 int bytes = camel_tcp_stream_wait (tcp_stream, stop_fd,
store-idle_sleep, line_buffer, 1023);
[snip]

I was thinking more along the lines of making the idle_loop use
PRPoll()/poll() itself - it already has to figure out what type of fd to
create for the cancel_idle_fd anyway.

 
 Afaics It's not true that you don't need to change any API:
 
 The camel_stream_read() will block until all n bytes are read.

no it doesn't. it returns as soon as any number of bytes are read or an
error occurs.

  This is
 something you don't know during IDLE (how many bytes you can expect to
 receive), and on top of that you want to continue immediately after
 select returns, not keep retrying until you have n bytes (to simulate a
 blocking read).

sure, but that's why you poll yourself. once the socket has data, /then/
you call camel_stream_read() on it.

  You also want a dedicated stop fildescriptor, not the
 standard cancel_fd as then any cancel will interfere with this (while
 you want strict control over this to let other threads start and stop
 IDLE).

right, but if you add a CamelStream API, then you have to add 1 for unix
fds and 1 for PRFileDesc

plus it keeps the code simpler (keeps all read() logic in the
camel_stream_read() implementations rather than duplicating it).


This actually brings me to another thought I've had for a few years now
but haven't bothered pushing for it... but it might be best if all of
the sockets used PRFileDesc rather than unix fd for raw sockets and
PRFileDesc for SSL sockets. If this is done, I think it'd be possible to
get rid of CamelTcpStreamRaw and move the implementation into
CamelTcpStream and make CamelTcpStreamSSL a simple subclass of
CamelTcpStream, replacing the CamelTcpStream's PRFileDesc with an
SSL-wrapped fd as appropriate and calling parent_class-read/write/etc
instead of implementing them all itself /again/.


Jeff


___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Let the porting begin

2007-10-24 Thread Jeffrey Stedfast
I took a look at the IDLE implementation last night and felt it went
about it the wrong way.

Firstly, the added camel_stream_[read,write]_nb() and
camel_stream_read_idle() functionality is totally unnecessary and just
makes the camel stream API gross (not to mention duplicating a lot of
code as there's no real difference from the normal read/write
implementations other than the timeout).

You should simply poll() on the socket descriptors (and a pipe fd used
for telling the IDLE thread's I/O loop to finish and send DONE).

Jeff

On Mon, 2007-10-08 at 00:41 +0200, Philip Van Hoof wrote:
 On Sun, 2007-10-07 at 14:15 +0200, Philip Van Hoof wrote:
  Hi there,
 
  Using this changeset you can follow the changes to camel-lite:
  
  http://tinymail.org/trac/tinymail/changeset/2823
  
 
 This changeset are a bunch of compilation warnings for Matthew's Base64
 patch to Camel: http://tinymail.org/trac/tinymail/changeset/2827
 
 
 ps. Adding Matthew in CC.
 

___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Let the porting begin

2007-10-24 Thread Philip Van Hoof
On Wed, 2007-10-24 at 11:58 -0400, Jeffrey Stedfast wrote:
 I took a look at the IDLE implementation last night and felt it went
 about it the wrong way.

Note that the IDLE implementation got changed (reimplemented) this
morning.

Although the same problems that you mention here still apply.

I also added some comments. The poll() idea is indeed something that at
some point I should refactor the loop to.

 Firstly, the added camel_stream_[read,write]_nb() and
 camel_stream_read_idle() functionality is totally unnecessary and just
 makes the camel stream API gross (not to mention duplicating a lot of
 code as there's no real difference from the normal read/write
 implementations other than the timeout).
 
 You should simply poll() on the socket descriptors (and a pipe fd used
 for telling the IDLE thread's I/O loop to finish and send DONE).
 
 Jeff
 
 On Mon, 2007-10-08 at 00:41 +0200, Philip Van Hoof wrote:
  On Sun, 2007-10-07 at 14:15 +0200, Philip Van Hoof wrote:
   Hi there,
  
   Using this changeset you can follow the changes to camel-lite:
   
 http://tinymail.org/trac/tinymail/changeset/2823
   
  
  This changeset are a bunch of compilation warnings for Matthew's Base64
  patch to Camel: http://tinymail.org/trac/tinymail/changeset/2827
  
  
  ps. Adding Matthew in CC.
  
 
-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://www.pvanhoof.be/blog




___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers


Re: [Evolution-hackers] Let the porting begin

2007-10-07 Thread Philip Van Hoof
On Sun, 2007-10-07 at 14:15 +0200, Philip Van Hoof wrote:
 Hi there,

 Using this changeset you can follow the changes to camel-lite:
 
   http://tinymail.org/trac/tinymail/changeset/2823

More such commits to Tinymail's camel-lite happened today. Nearly all of
those are merging work in action. After today, using KDiff3 or any other
tool that can recursively compare two directories should be painless and
straight forward on ./libtinymail-camel/camel-lite/camel
vs. ./evolution-data-server/camel


-- 
Philip Van Hoof, software developer
home: me at pvanhoof dot be 
gnome: pvanhoof at gnome dot org 
http://www.pvanhoof.be/blog




___
Evolution-hackers mailing list
Evolution-hackers@gnome.org
http://mail.gnome.org/mailman/listinfo/evolution-hackers