[balsa] Reduce buffer sizes used for imap compression to a reasonable size.

[Pawel Salek <pawels src gnome org>]

commit 734718660f5c511b1e4d7d46681d8ba7bf561344
Author: Pawel Salek <pawsa damage localdomain>
Date:   Tue Jan 5 23:22:32 2010 +0100

    Reduce buffer sizes used for imap compression to a reasonable size.
    
    * libbalsa/imap/siobuf.[ch]: change encode interface so that
      compression callback do not need to allocate too much memory.
    * libbalsa/imap/imap_compress.c: modify the callbacks accordingly.
    * libbalsa/imap/imap-handle.c: make sure we do not leave some
      unflushed compressed data around.
    * configure.in: zlib.h is unconditionally required.

 ChangeLog                     |    9 ++++++++
 libbalsa/imap/imap-handle.c   |   13 +++++++----
 libbalsa/imap/imap_compress.c |   44 +++++++++++++++++++---------------------
 libbalsa/imap/imap_tst.c      |   20 +++++++++---------
 libbalsa/imap/siobuf.c        |   37 ++++++++++++++++++++++++----------
 libbalsa/imap/siobuf.h        |    4 +-
 6 files changed, 76 insertions(+), 51 deletions(-)
---
diff --git a/ChangeLog b/ChangeLog
index 536c948..fc05906 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2010-01-05  Pawel Salek
+
+	* libbalsa/imap/siobuf.[ch]: change encode interface so that
+	compression callback do not need to allocate too much memory.
+	* libbalsa/imap/imap_compress.c: modify the callbacks accordingly.
+	* libbalsa/imap/imap-handle.c: make sure we do not leave some
+	unflushed compressed data around.
+	* configure.in: zlib.h is unconditionally required.
+
 2010-01-05  Peter Bloomfield
 
 	* libbalsa/html.c (libbalsa_html_new): remove dead code.
diff --git a/libbalsa/imap/imap-handle.c b/libbalsa/imap/imap-handle.c
index 3e10d7b..1202a60 100644
--- a/libbalsa/imap/imap-handle.c
+++ b/libbalsa/imap/imap-handle.c
@@ -167,7 +167,6 @@ imap_mbox_handle_init(ImapMboxHandle *handle)
   handle->enable_idle      = 1;
   mbox_view_init(&handle->mbox_view);
 
-  imap_compress_init(&handle->compress);
 #if defined(BALSA_USE_THREADS)
   pthread_mutex_init(&handle->mutex, NULL);
 #endif
@@ -559,6 +558,7 @@ imap_handle_disconnect(ImapMboxHandle *h)
   still_connected = imap_handle_idle_disable(h);
   if(h->sio) {
     sio_detach(h->sio); h->sio = NULL;
+    imap_compress_release(&h->compress);
   }
   if(h->iochannel) {
     g_io_channel_unref(h->iochannel); h->iochannel = NULL;
@@ -782,8 +782,8 @@ imap_mbox_connect(ImapMboxHandle* handle)
   handle->can_fetch_body = TRUE;
   handle->idle_state = IDLE_INACTIVE;
   if(handle->sio) {
-    sio_detach(handle->sio);
-    handle->sio = NULL;
+    sio_detach(handle->sio); handle->sio = NULL;
+    imap_compress_release(&handle->compress);
   }
 
 #ifdef USE_TLS
@@ -801,6 +801,7 @@ imap_mbox_connect(ImapMboxHandle* handle)
     close(handle->sd);
     return IMAP_NOMEM;
   }
+  imap_compress_init(&handle->compress);
   if(handle->timeout>0) {
     sio_set_timeout(handle->sio, handle->timeout);
     sio_set_timeoutcb(handle->sio, imap_timeout_cb, handle);
@@ -1018,7 +1019,6 @@ imap_mbox_handle_finalize(GObject* gobject)
   g_free(handle->msg_cache); handle->msg_cache = NULL;
   g_array_free(handle->flag_cache, TRUE);
 
-  imap_compress_release(&handle->compress);
   HANDLE_UNLOCK(handle);
 #if defined(BALSA_USE_THREADS)
   pthread_mutex_destroy(&handle->mutex);
@@ -2563,7 +2563,10 @@ ir_bye(ImapMboxHandle *h)
     imap_mbox_handle_set_msg(h, line);
     imap_mbox_handle_set_state(h, IMHS_DISCONNECTED);
     /* we close the connection here unless we are doing logout. */
-    if(h->sio) { sio_detach(h->sio); h->sio = NULL; }
+    if(h->sio) {
+      sio_detach(h->sio); h->sio = NULL; 
+      imap_compress_release(&h->compress);
+    }
     close(h->sd);
   }
   return IMR_BYE;
diff --git a/libbalsa/imap/imap_compress.c b/libbalsa/imap/imap_compress.c
index 10f0612..7c9d8de 100644
--- a/libbalsa/imap/imap_compress.c
+++ b/libbalsa/imap/imap_compress.c
@@ -25,10 +25,10 @@
 #include "imap-handle.h"
 #include "imap_private.h"
 
-/** Arbitrary - not really a useful one. */
-static const unsigned IMAP_COMPRESS_BUFFER_SIZE = 65536;
+/** Arbitrary. Current choice is as good as any other. */
+static const unsigned IMAP_COMPRESS_BUFFER_SIZE = 4096;
 
-static void
+static int
 imap_compress_cb(char **dstbuf, int *dstlen,
                  const char *srcbuf, int srclen, void *arg)
 {
@@ -37,33 +37,28 @@ imap_compress_cb(char **dstbuf, int *dstlen,
 
   *dstbuf = icb->out_buffer;
 
-  icb->out_stream.next_in  = (Bytef*)srcbuf;
-  icb->out_stream.avail_in = srclen;
+  if (*dstlen == 0) { /* first call */
+    icb->out_stream.next_in  = (Bytef*)srcbuf;
+    icb->out_stream.avail_in = srclen;
+    icb->out_uncompressed += srclen;
+  }
 
   icb->out_stream.next_out = (Byte*)*dstbuf;
   icb->out_stream.avail_out = IMAP_COMPRESS_BUFFER_SIZE;
 
-  do {
-    if (icb->out_stream.avail_out ==0){
-      printf("Buffer reallocation not implemented, aborting.\n");
-      /* FIXME1 */
-      break;
-    }
-    /* check sizes here */
-    err = deflate(&icb->out_stream, Z_SYNC_FLUSH);
-    if ( !(err == Z_OK || err == Z_STREAM_END || err == Z_BUF_ERROR) ) {
-      fprintf(stderr, "deflate error1 %d\n", err);
-      /* FIXME - break the connection here, no point in continuing. */
-    }
-  } while (icb->out_stream.avail_out == 0);
+  err = deflate(&icb->out_stream, Z_SYNC_FLUSH);
+  if ( !(err == Z_OK || err == Z_STREAM_END || err == Z_BUF_ERROR) ) {
+    fprintf(stderr, "deflate error1 %d\n", err);
+    /* FIXME - break the connection here, no point in continuing. */
+  }
 
   *dstlen = IMAP_COMPRESS_BUFFER_SIZE - icb->out_stream.avail_out;
   /* printf("imap_compress_cb %d bytes to %d\n", srclen, *dstlen); */
-  icb->out_uncompressed += srclen;
   icb->out_compressed += *dstlen;
+  return *dstlen;
 }
 
-static void
+static int
 imap_decompress_cb(char **dstbuf, int *dstlen,
                    const char *srcbuf, int srclen, void *arg)
 {
@@ -72,8 +67,11 @@ imap_decompress_cb(char **dstbuf, int *dstlen,
 
   *dstbuf = icb->in_buffer;
 
-  icb->in_stream.next_in  = (Bytef*)srcbuf;
-  icb->in_stream.avail_in = srclen;
+  if (srclen) {
+    icb->in_stream.next_in  = (Bytef*)srcbuf;
+    icb->in_stream.avail_in = srclen;
+    icb->in_compressed += srclen;
+  }
 
   icb->in_stream.next_out = (Byte*)*dstbuf;
   icb->in_stream.avail_out =  IMAP_COMPRESS_BUFFER_SIZE;
@@ -86,8 +84,8 @@ imap_decompress_cb(char **dstbuf, int *dstlen,
 
   *dstlen = IMAP_COMPRESS_BUFFER_SIZE - icb->in_stream.avail_out;
   /* printf("imap_decompress_cb %d bytes to %d\n", srclen, *dstlen); */
-  icb->in_compressed += srclen;
   icb->in_uncompressed += *dstlen;
+  return *dstlen;
 }
 
 /** Enables COMPRESS extension if available. Assumes that the handle
diff --git a/libbalsa/imap/imap_tst.c b/libbalsa/imap/imap_tst.c
index 8f6767b..e47490b 100644
--- a/libbalsa/imap/imap_tst.c
+++ b/libbalsa/imap/imap_tst.c
@@ -626,18 +626,18 @@ process_options(int argc, char *argv[])
     } else if( strcmp(argv[first_arg], "-p") == 0 &&
 	       first_arg+1 < argc) {
       TestContext.password = argv[++first_arg];
+    } else if( strcmp(argv[first_arg], "-a") == 0) {
+      TestContext.anonymous = TRUE;
+    } else if( strcmp(argv[first_arg], "-c") == 0) {
+      TestContext.compress = TRUE;
     } else if( strcmp(argv[first_arg], "-m") == 0) {
       TestContext.monitor = TRUE;
+    } else if( strcmp(argv[first_arg], "-s") == 0) {
+      TestContext.over_ssl = TRUE;
     } else if( strcmp(argv[first_arg], "-T") == 0) {
       TestContext.tls_mode = IMAP_TLS_REQUIRED;
     } else if( strcmp(argv[first_arg], "-t") == 0) {
       TestContext.tls_mode = IMAP_TLS_DISABLED;
-    } else if( strcmp(argv[first_arg], "-s") == 0) {
-      TestContext.over_ssl = TRUE;
-    }  else if( strcmp(argv[first_arg], "-a") == 0) {
-      TestContext.anonymous = TRUE;
-    }  else if( strcmp(argv[first_arg], "-c") == 0) {
-      TestContext.compress = TRUE;
     } else {
       break; /* break the loop - non-option encountered. */
     }
@@ -679,12 +679,12 @@ main(int argc, char *argv[]) {
     fprintf(stderr, "Known options:\n"
             "-u USER specify user\n"
             "-p PASSWORD specify password\n"
+            "-a anonymous\n"
+            "-c compress\n"
             "-m enable monitor\n"
-            "-T tls required\n"
-            "-t tls disabled\n"
             "-s over ssl\n"
-            "-a anonymous\n"
-            "-c compress\n");
+            "-T tls required\n"
+            "-t tls disabled\n");
 
     return 1;
   }
diff --git a/libbalsa/imap/siobuf.c b/libbalsa/imap/siobuf.c
index 2080044..619cc5c 100644
--- a/libbalsa/imap/siobuf.c
+++ b/libbalsa/imap/siobuf.c
@@ -460,7 +460,7 @@ sio_flush (struct siobuf *sio)
   if (sio->encode_cb != NULL)
     {
       char *buf;
-      int len;
+      int len = 0;
 
       /* Rules for the encode callback.
 
@@ -470,8 +470,10 @@ sio_flush (struct siobuf *sio)
          callback must maintain its own buffer which must persist until
          the next call in the same thread.  The secarg argument may be
          used to maintain this buffer. */
-      (*sio->encode_cb) (&buf, &len, sio->write_buffer, length, sio->secarg);
-      raw_write (sio, buf, len);
+      while ((*sio->encode_cb) (&buf, &len, sio->write_buffer, 
+                                length, sio->secarg)) {
+        raw_write (sio, buf, len);
+      }
     }
   else
     raw_write (sio, sio->write_buffer, length);
@@ -557,11 +559,8 @@ sio_fill (struct siobuf *sio)
 {
   assert (sio != NULL);
 
-  sio->read_unread = raw_read (sio, sio->read_buffer, sio->buffer_size);
-  if (sio->read_unread <= 0)
-    return 0;
 
-  if (sio->decode_cb != NULL)
+  if (sio->decode_cb != NULL) {
     /* Rules for the decode callback.
 
        The output variables (here buf and len) may be set to the
@@ -569,11 +568,27 @@ sio_fill (struct siobuf *sio)
        the result is shorter than the original data.  Otherwise the
        callback must maintain its own buffer which must persist until
        the next call in the same thread.  The secarg argument may be
-       used to maintain this buffer. */
-    (*sio->decode_cb) (&sio->read_position, &sio->read_unread,
-		       sio->read_buffer, sio->read_unread, sio->secarg);
-  else
+       used to maintain this buffer.
+       
+       Decode callback gets at most twice for each buffer: first time
+       with buflen == 0 to decode whatever is in the internal decode
+       buffer. If that call returns 0, actual data read is performed
+       and decode is given the second shot, when it isupposed to
+       return nonzero.
+    */
+    while (!(*sio->decode_cb) (&sio->read_position, &sio->read_unread,
+                               sio->read_buffer, sio->read_unread,
+                               sio->secarg)) {
+      sio->read_unread = raw_read (sio, sio->read_buffer, sio->buffer_size);
+    }
+    if (sio->read_unread <= 0)
+      return 0;
+  } else {
+    sio->read_unread = raw_read (sio, sio->read_buffer, sio->buffer_size);
+    if (sio->read_unread <= 0)
+      return 0;
     sio->read_position = sio->read_buffer;
+  }
 
   if (sio->monitor_cb != NULL && sio->read_unread > 0)
     (*sio->monitor_cb) (sio->read_position, sio->read_unread,
diff --git a/libbalsa/imap/siobuf.h b/libbalsa/imap/siobuf.h
index a280f4b..656d251 100644
--- a/libbalsa/imap/siobuf.h
+++ b/libbalsa/imap/siobuf.h
@@ -28,8 +28,8 @@ typedef struct siobuf *siobuf_t;
 #define SIO_READ	1
 #define SIO_WRITE	2
 
-typedef void (*recodecb_t) (char **dstbuf, int *dstlen,
-			    const char *srcbuf, int srclen, void *arg);
+typedef int (*recodecb_t) (char **dstbuf, int *dstlen,
+                           const char *srcbuf, int srclen, void *arg);
 typedef void (*monitorcb_t) (const char *buffer, int length, int direction,
 			     void *arg);
 typedef int (*timeoutcb_t) (void *arg);