From c80be5359a5093807a403b03371f35e2895e4087 Mon Sep 17 00:00:00 2001
From: vb <devnull@localhost>
Date: Sun, 22 Feb 2015 13:40:25 +0100
Subject: [PATCH] some redesign and bugfixing

---
 src/message_api.c        | 244 +++++++++++++++------------------------
 src/mime.c               |  47 +++-----
 src/pgp_gpg.c            |  30 ++---
 src/transport.c          |  28 ++---
 test/message_api_test.cc |  17 +--
 5 files changed, 137 insertions(+), 229 deletions(-)

diff --git a/src/message_api.c b/src/message_api.c
index 730c04f3..b2b0d85e 100644
--- a/src/message_api.c
+++ b/src/message_api.c
@@ -11,18 +11,23 @@
 static char * combine_short_and_long(const message * src)
 {
     char * ptext;
+    char * longmsg;
     assert(src);
-    assert(src->shortmsg && src->longmsg && strcmp(src->shortmsg, "pEp") != 0);
+    assert(src->shortmsg && strcmp(src->shortmsg, "pEp") != 0);
 
-    ptext = calloc(1, strlen(src->shortmsg) + strlen(src->longmsg)
-            + 12);
+    if (src->longmsg)
+        longmsg = src->longmsg;
+    else
+        longmsg = "";
+
+    ptext = calloc(1, strlen(src->shortmsg) + strlen(longmsg) + 12);
     if (ptext == NULL)
         return NULL;
 
     strcpy(ptext, "subject: ");
     strcat(ptext, src->shortmsg);
     strcat(ptext, "\n\n");
-    strcat(ptext, src->longmsg);
+    strcat(ptext, longmsg);
 
     return ptext;
 }
@@ -36,6 +41,8 @@ DYNAMIC_API PEP_STATUS encrypt_message(
     )
 {
     PEP_STATUS status = PEP_STATUS_OK;
+    message * msg = NULL;
+    stringlist_t * keys = NULL;
 
     assert(session);
     assert(src);
@@ -45,64 +52,51 @@ DYNAMIC_API PEP_STATUS encrypt_message(
 
     pEp_identity *from = identity_dup(src->from);
     if (from == NULL)
-        return PEP_OUT_OF_MEMORY;
+        goto enomem;
+    from->me = true;
 
     identity_list *to = identity_list_dup(src->to);
     if (to == NULL) {
         free_identity(from);
-        return PEP_OUT_OF_MEMORY;
+        goto enomem;
     }
 
-    message *msg = new_message(src->dir, from, to, NULL);
+    msg = new_message(src->dir, from, to, NULL);
     if (msg == NULL) {
         free_identity(from);
         free_identity_list(to);
-        return PEP_OUT_OF_MEMORY;
+        goto enomem;
     }
     msg->enc_format = PEP_enc_pieces;
 
-    from->me = true;
-
     status = myself(session, from);
-    if (status != PEP_STATUS_OK) {
-        free_message(msg);
-        return status;
-    }
+    if (status != PEP_STATUS_OK)
+        goto pep_error;
 
-    stringlist_t * keys = new_stringlist(from->fpr);
-    if (keys == NULL) {
-        free_message(msg);
-        return PEP_OUT_OF_MEMORY;
-    }
+    keys = new_stringlist(from->fpr);
+    if (keys == NULL)
+        goto enomem;
 
     stringlist_t *_k = keys;
 
     if (extra) {
         _k = stringlist_append(_k, extra);
-        if (_k == NULL) {
-            free_stringlist(keys);
-            free_message(msg);
-            return PEP_OUT_OF_MEMORY;
-        }
+        if (_k == NULL)
+            goto enomem;
     }
 
     bool dest_keys_found = false;
     identity_list * _il;
     for (_il = to; _il && _il->ident; _il = _il->next) {
-        PEP_STATUS _status = update_identity(session, _il->ident);
-        if (_status != PEP_STATUS_OK) {
-            free_message(msg);
-            free_stringlist(keys);
-            return _status;
-        }
+        PEP_STATUS status = update_identity(session, _il->ident);
+        if (status != PEP_STATUS_OK)
+            goto pep_error;
+
         if (_il->ident->fpr) {
             dest_keys_found = true;
             _k = stringlist_add(_k, _il->ident->fpr);
-            if (_k == NULL) {
-                free_message(msg);
-                free_stringlist(keys);
-                return PEP_OUT_OF_MEMORY;
-            }
+            if (_k == NULL)
+                goto enomem;
         }
         else
             status = PEP_KEY_NOT_FOUND;
@@ -116,107 +110,66 @@ DYNAMIC_API PEP_STATUS encrypt_message(
 
         switch (format) {
         case PEP_enc_MIME_multipart: {
-            char *resulttext;
+            char *resulttext = NULL;
             bool free_ptext = false;
             msg->enc_format = PEP_enc_MIME_multipart;
 
-            if (src->shortmsg && src->longmsg && strcmp(src->shortmsg, "pEp") != 0) {
+            if (src->shortmsg && strcmp(src->shortmsg, "pEp") != 0) {
                 ptext = combine_short_and_long(src);
-                if (ptext == NULL) {
-                    free_message(msg);
-                    free_stringlist(keys);
-                    return PEP_OUT_OF_MEMORY;
-                }
+                if (ptext == NULL)
+                    goto enomem;
                 free_ptext = true;
             }
             else if (src->longmsg) {
                 ptext = src->longmsg;
             }
             else {
-                ptext = NULL;
+                assert(0);
+                status = PEP_ILLEGAL_VALUE;
+                goto pep_error;
             }
 
-            // TO EXTEND: we only support HTML yet
-            assert(src->format == PEP_format_plain
-                    || src->format == PEP_format_html);
-
             status = mime_encode_text(ptext, src->longmsg_formatted,
                     src->attachments, &resulttext);
             assert(status == PEP_STATUS_OK);
             if (free_ptext)
                 free(ptext);
             assert(resulttext);
-            if (resulttext == NULL) {
-                free_message(msg);
-                free_stringlist(keys);
-                return status;
-            }
+            if (resulttext == NULL)
+                goto pep_error;
             
             status = encrypt_and_sign(session, keys, resulttext, strlen(resulttext),
                     &ctext, &csize);
             free(resulttext);
-            free_stringlist(keys);
             if (ctext) {
                 msg->longmsg = strdup(ctext);
                 msg->shortmsg = strdup("pEp");
-                if (!(msg->longmsg && msg->shortmsg)) {
-                    free_message(msg);
-                    return PEP_OUT_OF_MEMORY;
-                }
+                if (!(msg->longmsg && msg->shortmsg))
+                    goto enomem;
+            }
+            else {
+                goto pep_error;
             }
         }
+        break;
 
         case PEP_enc_pieces:
-            if (src->shortmsg && src->longmsg && strcmp(src->shortmsg, "pEp") != 0) {
+            if (src->shortmsg && strcmp(src->shortmsg, "pEp") != 0) {
                 ptext = combine_short_and_long(src);
-                if (ptext == NULL) {
-                    free_message(msg);
-                    free_stringlist(keys);
-                    return PEP_OUT_OF_MEMORY;
-                }
-                status = encrypt_and_sign(session, keys, ptext, strlen(ptext),
-                        &ctext, &csize);
-                free(ptext);
-                if (ctext) {
-                    msg->longmsg = strdup(ctext);
-                    msg->shortmsg = strdup("pEp");
-                    if (!(msg->longmsg && msg->shortmsg)) {
-                        free_stringlist(keys);
-                        free_message(msg);
-                        return PEP_OUT_OF_MEMORY;
-                    }
-                }
-                else {
-                    free_message(msg);
-                    free_stringlist(keys);
-                    msg = NULL;
-                }
-            }
-            else if (src->shortmsg && strcmp(src->shortmsg, "pEp") != 0) {
-                ptext = calloc(1, strlen(src->shortmsg) + 12);
-                if (ptext == NULL) {
-                    free_message(msg);
-                    free_stringlist(keys);
-                    return PEP_OUT_OF_MEMORY;
-                }
-                strcpy(ptext, "subject: ");
-                strcat(ptext, src->shortmsg);
-                strcat(ptext, "\n\n");
+                if (ptext == NULL)
+                    goto enomem;
+
                 status = encrypt_and_sign(session, keys, ptext, strlen(ptext),
                         &ctext, &csize);
                 free(ptext);
                 if (ctext) {
                     msg->longmsg = strdup(ctext);
                     msg->shortmsg = strdup("pEp");
-                    if (!(msg->longmsg && msg->shortmsg)) {
-                        free_message(msg);
-                        free_stringlist(keys);
-                        return PEP_OUT_OF_MEMORY;
-                    }
+                    if (!(msg->longmsg && msg->shortmsg))
+                        goto enomem;
                 }
                 else {
-                    free_message(msg);
-                    msg = NULL;
+                    goto pep_error;
                 }
             }
             else if (src->longmsg) {
@@ -226,81 +179,76 @@ DYNAMIC_API PEP_STATUS encrypt_message(
                 if (ctext) {
                     msg->longmsg = strdup(ctext);
                     msg->shortmsg = strdup("pEp");
-                    if (!(msg->longmsg && msg->shortmsg)) {
-                        free_message(msg);
-                        return PEP_OUT_OF_MEMORY;
-                    }
+                    if (!(msg->longmsg && msg->shortmsg))
+                        goto enomem;
                 }
                 else {
-                    free_message(msg);
-                    msg = NULL;
+                    goto pep_error;
                 }
             }
-            if (msg && msg->longmsg_formatted) {
+
+            if (msg->longmsg_formatted) {
                 ptext = src->longmsg_formatted;
                 status = encrypt_and_sign(session, keys, ptext, strlen(ptext),
                         &ctext, &csize);
                 if (ctext) {
                     msg->longmsg_formatted = strdup(ctext);
-                    if (msg->longmsg_formatted == NULL) {
-                        free_message(msg);
-                        return PEP_OUT_OF_MEMORY;
-                    }
+                    if (msg->longmsg_formatted == NULL)
+                        goto enomem;
                 }
                 else {
-                    free_message(msg);
-                    msg = NULL;
+                    goto pep_error;
                 }
             }
-            if (msg) {
-                if (src->attachments) {
-                    bloblist_t *_s;
-                    bloblist_t *_d = new_bloblist(NULL, 0, NULL, NULL);
-                    if (_d == NULL) {
-                        free_message(msg);
-                        free_stringlist(keys);
-                        return PEP_OUT_OF_MEMORY;
+
+            if (src->attachments) {
+                bloblist_t *_s;
+                bloblist_t *_d = new_bloblist(NULL, 0, NULL, NULL);
+                if (_d == NULL)
+                    goto enomem;
+
+                msg->attachments = _d;
+                for (_s = src->attachments; _s && _s->data; _s = _s->next) {
+                    int psize = _s->size;
+                    ptext = _s->data;
+                    status = encrypt_and_sign(session, keys, ptext, psize,
+                            &ctext, &csize);
+                    if (ctext) {
+                        char * _c = strdup(ctext);
+                        if (_c == NULL)
+                            goto enomem;
+
+                        _d = bloblist_add(_d, _c, csize, _s->mime_type,
+                                _s->file_name);
+                        if (_d == NULL)
+                            goto enomem;
                     }
-                    msg->attachments = _d;
-                    for (_s = src->attachments; _s && _s->data; _s = _s->next) {
-                        int psize = _s->size;
-                        ptext = _s->data;
-                        status = encrypt_and_sign(session, keys, ptext, psize,
-                                &ctext, &csize);
-                        if (ctext) {
-                            char * _c = strdup(ctext);
-                            if (_c == NULL) {
-                                free_message(msg);
-                                free_stringlist(keys);
-                                return PEP_OUT_OF_MEMORY;
-                            }
-                            _d = bloblist_add(_d, _c, csize, _s->mime_type,
-                                    _s->file_name);
-                            if (_d == NULL) {
-                                free_message(msg);
-                                free_stringlist(keys);
-                                return PEP_OUT_OF_MEMORY;
-                            }
-                        }
-                        else {
-                            free_message(msg);
-                            msg = NULL;
-                            break;
-                        }
+                    else {
+                        goto pep_error;
                     }
                 }
-                *dst = msg;
             }
             break;
 
         default:
             assert(0);
+            status = PEP_ILLEGAL_VALUE;
+            goto pep_error;
         }
     }
-    else
-        free_message(msg);
 
     free_stringlist(keys);
+
+    *dst = msg;
+    return PEP_STATUS_OK;
+
+enomem:
+    status = PEP_OUT_OF_MEMORY;
+
+pep_error:
+    free_stringlist(keys);
+    free_message(msg);
+
     return status;
 }
 
diff --git a/src/mime.c b/src/mime.c
index 6ca97372..903b2d65 100644
--- a/src/mime.c
+++ b/src/mime.c
@@ -8,6 +8,7 @@
 #include <unistd.h>
 
 #include "etpan_mime.h"
+#include "wrappers.h"
 
 DYNAMIC_API PEP_STATUS mime_encode_text(
         const char *plaintext,
@@ -127,10 +128,7 @@ DYNAMIC_API PEP_STATUS mime_encode_text(
     if (template == NULL)
         goto enomem;
 
-    do {
-        fd = mkstemp(template);
-    } while (fd == -1 && errno == EINTR);
-
+    fd = Mkstemp(template);
     assert(fd != -1);
     if (fd == -1)
         goto err_file;
@@ -143,10 +141,7 @@ DYNAMIC_API PEP_STATUS mime_encode_text(
     free(template);
     template = NULL;
 
-    do {
-        file = fdopen(fd, "w+");
-    } while (file == NULL && errno == EINTR);
-
+    file = Fdopen(fd, "w+");
     assert(file);
     if (file == NULL) {
         switch (errno) {
@@ -179,38 +174,28 @@ DYNAMIC_API PEP_STATUS mime_encode_text(
 
     errno = 0;
     rewind(file);
-
     assert(errno == 0);
+#ifdef NDEBUG
     switch (errno) {
+        case 0:
+            break;
         case ENOMEM:
             goto enomem;
         default:
             goto err_file;
     }
+#endif
 
     buf = calloc(1, size + 1);
     assert(buf);
     if (buf == NULL)
         goto enomem;
-    
-    char *_buf = buf;
-    size_t rest = size;
-    for (size_t bytes_read = 0; rest > 0; rest -= bytes_read, _buf += rest) {
-        clearerr(file);
-        bytes_read = rest * fread(_buf, rest, 1, file);
-
-        assert(ferror(file) == 0 || ferror(file) == EINTR);
-        if (ferror(file) != 0 && ferror(file) != EINTR)
-            goto err_file;
-
-        assert(!feof(file));
-        if (feof(file))
-            goto err_file;
-    }
+ 
+    size_t _read;
+    _read = Fread1(buf, size, file);
+    assert(_read == size);
 
-    do {
-        r = fclose(file);
-    } while (r == -1 && errno == EINTR);
+    r = Fclose(file);
     assert(r == 0);
 
     mailmime_free(mime);
@@ -233,15 +218,11 @@ release:
     free(template);
 
     if (file) {
-        do {
-            r = fclose(file);
-        } while (r == -1 && errno == EINTR);
+        r = Fclose(file);
         assert(r == 0);
     }
     else if (fd != -1) {
-        do {
-            r = close(fd);
-        } while (r == -1 && errno == EINTR);
+        r = Close(fd);
         assert(r == 0);
     }
 
diff --git a/src/pgp_gpg.c b/src/pgp_gpg.c
index 94cf3e2b..779aa382 100644
--- a/src/pgp_gpg.c
+++ b/src/pgp_gpg.c
@@ -1,5 +1,3 @@
-#include <errno.h>
-
 #include "pgp_gpg.h"
 #include "pEp_internal.h"
 
@@ -15,17 +13,15 @@ static bool ensure_keyserver()
     int r;
 
     f = Fopen(gpg_conf(), "r");
-    if (errno == ENOMEM)
+    assert(f);
+    if (f == NULL && errno == ENOMEM)
         return false;
 
     if (f != NULL) {
         do {
             char * s;
 
-            do {
-                s = fgets(buf, MAX_LINELENGTH, f);
-            } while (s == NULL && !feof(f) && errno == EINTR);
-
+            s = Fgets(buf, MAX_LINELENGTH, f);
             assert(s);
             if (s == NULL)
                 return false;
@@ -33,36 +29,26 @@ static bool ensure_keyserver()
             if (s && !feof(f)) {
                 char * t = strtok(s, " ");
                 if (t && strcmp(t, "keyserver") == 0) {
-                    do {
-                        r = fclose(f);
-                    } while (r == -1 && errno == EINTR);
+                    r = Fclose(f);
                     assert(r == 0);
                     return true;
                 }
             }
         } while (!feof(f));
-        do {
-            f = freopen(gpg_conf(), "a", f);
-        } while (f == NULL && errno == EINTR);
+        f = Freopen(gpg_conf(), "a", f);
     }
     else {
-        do {
-            f = fopen(gpg_conf(), "w");
-        } while (f == NULL && errno == EINTR);
+        f = Fopen(gpg_conf(), "w");
     }
 
     assert(f);
     if (f == NULL)
         return false;
 
-    do {
-        n = fprintf(f, "keyserver %s\n", DEFAULT_KEYSERVER);
-    } while (n < 0 && errno == EINTR);
+    n = Fprintf(f, "keyserver %s\n", DEFAULT_KEYSERVER);
     assert(n >= 0);
 
-    do {
-        r = fclose(f);
-    } while (r == -1 && errno == EINTR);
+    r = Fclose(f);
     assert(r == 0);
 
     return true;
diff --git a/src/transport.c b/src/transport.c
index 9d2d876e..2b8dae01 100644
--- a/src/transport.c
+++ b/src/transport.c
@@ -197,19 +197,21 @@ DYNAMIC_API message *new_message(
 
 DYNAMIC_API void free_message(message *msg)
 {
-    free(msg->id);
-    free(msg->shortmsg);
-    free(msg->longmsg);
-    free(msg->longmsg_formatted);
-    free_bloblist(msg->attachments);
-    free_identity(msg->from);
-    free_identity_list(msg->to);
-    free_identity(msg->recv_by);
-    free_identity_list(msg->cc);
-    free_identity_list(msg->bcc);
-    free(msg->refering_id);
-    free_message_ref_list(msg->refered_by);
-    free(msg);
+    if (msg) {
+        free(msg->id);
+        free(msg->shortmsg);
+        free(msg->longmsg);
+        free(msg->longmsg_formatted);
+        free_bloblist(msg->attachments);
+        free_identity(msg->from);
+        free_identity_list(msg->to);
+        free_identity(msg->recv_by);
+        free_identity_list(msg->cc);
+        free_identity_list(msg->bcc);
+        free(msg->refering_id);
+        free_message_ref_list(msg->refered_by);
+        free(msg);
+    }
 }
 
 DYNAMIC_API message_ref_list *new_message_ref_list(message *msg)
diff --git a/test/message_api_test.cc b/test/message_api_test.cc
index b5b18244..88660429 100644
--- a/test/message_api_test.cc
+++ b/test/message_api_test.cc
@@ -25,26 +25,17 @@ int main() {
     assert(msg);
     cout << "message created.\n";
 
-//     cout << "encrypting message in pieces…\n";
-//     message *enc_msg;
-//     cout << "calling encrypt_message()\n";
-//     PEP_STATUS status2 = encrypt_message(session, msg, NULL, &enc_msg, PEP_enc_pieces);
-//     assert(status2 == PEP_STATUS_OK);
-//     assert(enc_msg);
-//     cout << "message encrypted.\n";
-
     cout << "encrypting message as MIME multipart…\n";
-    message *enc_msg2;
+    message *enc_msg3;
     cout << "calling encrypt_message()\n";
-    PEP_STATUS status3 = encrypt_message(session, msg, NULL, &enc_msg2, PEP_enc_MIME_multipart);
+    PEP_STATUS status3 = encrypt_message(session, msg, NULL, &enc_msg3, PEP_enc_MIME_multipart);
     assert(status3 == PEP_STATUS_OK);
-    assert(enc_msg2);
+    assert(enc_msg3);
     cout << "message encrypted.\n";
+    free_message(enc_msg3);
     
     cout << "freeing messages…\n";
     free_message(msg);
-//    free_message(enc_msg);
-    free_message(enc_msg2);
     cout << "done.\n";
 
     cout << "calling release()\n";