Refactor patty_kiss_tnc_recv()

Refactor patty_kiss_tnc_recv() to be much more compact and correct;
ensure the KISS command byte is absolutely required, and that
consecutive frame end markers are handled without error

include/patty/kiss.h

@@ -35,20 +35,15 @@ int patty_kiss_tnc_fd_unix(patty_kiss_tnc *tnc);
 
 void patty_kiss_tnc_close(patty_kiss_tnc *tnc);
 
-void patty_kiss_tnc_drop(patty_kiss_tnc *tnc);
-
 size_t patty_kiss_tnc_dropped(patty_kiss_tnc *tnc);
 
-void patty_kiss_tnc_flush(patty_kiss_tnc *tnc, size_t len);
-
-ssize_t patty_kiss_tnc_buffer(patty_kiss_tnc *tnc);
-
-ssize_t patty_kiss_tnc_decode(patty_kiss_tnc *tnc, void *frame, size_t *len, int *port);
-
 ssize_t patty_kiss_tnc_recv(patty_kiss_tnc *tnc,
-    void **frame, int *port);
+                            void **frame,
+                            int *port);
 
 ssize_t patty_kiss_tnc_send(patty_kiss_tnc *tnc,
-    const void *frame, size_t len, int port);
+                            const void *frame,
+                            size_t len,
+                            int port);
 
 #endif /* _PATTY_KISS_H */

src/kiss.c

@@ -11,9 +11,10 @@
 #include <patty/kiss.h>
 
 enum kiss_flags {
-    KISS_NONE   = 0x00,
+    KISS_NONE    = 0,
-    KISS_FRAME  = 0x01,
+    KISS_FRAME   = 1 << 0,
-    KISS_ESCAPE = 0x02
+    KISS_COMMAND = 1 << 1,
+    KISS_ESCAPE  = 1 << 2
 };
 
 struct _patty_kiss_tnc {
@@ -21,8 +22,8 @@ struct _patty_kiss_tnc {
 
     int fd;
 
-    void *frame,
+    void *buf,
-         *buf;
+         *frame;
 
     size_t bufsz,
            buflen,
@@ -79,132 +80,35 @@ void patty_kiss_tnc_close(patty_kiss_tnc *tnc) {
      free(tnc);
 }
 
-ssize_t patty_kiss_tnc_buffer(patty_kiss_tnc *tnc) {
+static ssize_t tnc_buffer(patty_kiss_tnc *tnc) {
-     size_t fillsz  = tnc->bufsz - tnc->buflen;
+    ssize_t readlen;
-    ssize_t readlen = 0;
 
-    /*
+    if (tnc->buflen == tnc->bufsz) {
-     * If the buffer needs to be filled, then fill it.
+        goto full;
-     */
-    if (fillsz) {
-        void *dest = ((uint8_t *)tnc->buf) + tnc->buflen;
-
-        if ((readlen = read(tnc->fd, dest, fillsz)) < 0) {
-            errno = EIO;
-
-            goto error_io;
-        }
-
-        tnc->buflen += readlen;
     }
 
+    if ((readlen = read(tnc->fd, tnc->buf, tnc->bufsz - tnc->buflen)) < 0) {
+        goto error_read;
+    }
+
+    tnc->buflen += readlen;
+
     return readlen;
 
-error_io:
+full:
+    return 0;
+
+error_read:
     return -1;
 }
 
-ssize_t patty_kiss_tnc_decode(patty_kiss_tnc *tnc, void *frame, size_t *len, int *port) {
+static void tnc_flush(patty_kiss_tnc *tnc, size_t len) {
-    size_t i, b;
+    memmove(tnc->buf, ((uint8_t *)tnc->buf) + len, tnc->buflen - len);
-    enum kiss_flags flags = KISS_NONE;
 
-    /*
+    tnc->buflen -= len;
-     * The default port is always zero, unless a command byte issued from the
-     * TNC in the first byte of the frame body.
-     */
-    *port = 0;
-
-    /*
-     * Ah, yes, the icky state machine wherein we unescape things off the wire
-     * and whatnot.
-     */
-    for (i=0, b=0; i<tnc->buflen; i++) {
-        uint8_t c = ((uint8_t *)tnc->buf)[i];
-
-        if (i == 0) {
-            if (c != PATTY_KISS_FEND) {
-                /*
-                 * If the first byte is not a frame end, then that's a bad thing.
-                 */
-                errno = EIO;
-
-                goto error_io;
-            }
-        } else if (i == 1) {
-            if (PATTY_KISS_COMMAND(c) != PATTY_KISS_DATA) {
-                errno = EIO;
-
-                goto error_io;
-            }
-
-            *port = PATTY_KISS_COMMAND_PORT(c);
-
-            continue;
-        }
-
-        if ((flags & KISS_FRAME) == 0) {
-            if (c == PATTY_KISS_FEND) {
-                flags |= KISS_FRAME;
-
-                continue;
-            }
-
-            errno = EIO;
-
-            goto error_io;
-        }
-
-        if (flags & KISS_ESCAPE) {
-            if (c == PATTY_KISS_TFEND) {
-                ((uint8_t *)frame)[b++] = PATTY_KISS_FEND;
-            } else if (c == PATTY_KISS_TFESC) {
-                ((uint8_t *)frame)[b++] = PATTY_KISS_FESC;
-            } else {
-                errno = EIO;
-
-                goto error_io;
-            }
-
-            flags &= ~KISS_ESCAPE;
-
-            continue;
-        }
-
-        if (flags & KISS_FRAME) {
-            if (c == PATTY_KISS_FESC) {
-                flags |= KISS_ESCAPE;
-
-                continue;
-            } else if (c == PATTY_KISS_FEND) {
-                break;
-            }
-
-            ((uint8_t *)frame)[b++] = c;
-        }
-    }
-
-    /*
-     * Return empty-handed if we do not have a fully book-ended frame.
-     */
-    if ((flags & KISS_FRAME) == 0) {
-        *len  = 0;
-        *port = 0;
-
-        return 0;
-    }
-
-    *len = b;
-
-    return i;
-
-error_io:
-    return -1;
 }
 
-void patty_kiss_tnc_drop(patty_kiss_tnc *tnc) {
+static void tnc_drop(patty_kiss_tnc *tnc) {
-    memset(tnc->buf,   '\0', tnc->bufsz);
-    memset(tnc->frame, '\0', tnc->bufsz);
-
     tnc->buflen = 0;
     tnc->dropped++;
 }
@@ -213,63 +117,97 @@ size_t patty_kiss_tnc_dropped(patty_kiss_tnc *tnc) {
     return tnc->dropped;
 }
 
-void patty_kiss_tnc_flush(patty_kiss_tnc *tnc, size_t len) {
+ssize_t patty_kiss_tnc_recv(patty_kiss_tnc *tnc,
-    /*
+                            void **frame,
-     * Move everything from the buffer not processed up to this point, to the
+                            int *port) {
-     * beginning of the buffer.
+    size_t i = 0,
-     */
+           b = 0;
-    memmove(tnc->buf, ((uint8_t *)tnc->buf) + len, tnc->buflen - len);
 
-    /*
+    enum kiss_flags flags = KISS_NONE;
-     * Then, decrement the buffer length by the number of bytes already
-     * processed.
-     */
-    tnc->buflen -= len;
-}
 
-ssize_t patty_kiss_tnc_recv(patty_kiss_tnc *tnc, void **frame, int *port) {
+    if (tnc_buffer(tnc) < 0) {
-    ssize_t buffered, decoded;
+        goto error_io;
-     size_t framelen;
+    }
 
-    /*
+    while (i < tnc->buflen) {
-     * Initialize the frame to be returned to the caller.
+        uint8_t c = ((uint8_t *)tnc->buf)[i++];
-     */
-    memset(tnc->frame, '\0', tnc->bufsz);
 
-    do {
+        if (!(flags & KISS_FRAME)) {
-        /*
+            if (c == PATTY_KISS_FEND) {
-         * Fill the buffer with something to parse.
+                flags |= KISS_FRAME;
-         */
+
-        if ((buffered = patty_kiss_tnc_buffer(tnc)) < 0) {
+                continue;
-            goto error_io;
+            } else {
+                errno = EIO;
+
+                goto error_io;
+            }
+        } else {
+            if (c == PATTY_KISS_FEND) {
+                if (b > 0) {
+                    flags &= ~KISS_FRAME;
+
+                    goto done;
+                }
+
+                continue;
+            }
         }
 
-        /*
+        if (!(flags & KISS_COMMAND)) {
-         * Try to decode the TNC buffer into the frame.
+            if (PATTY_KISS_COMMAND(c) != PATTY_KISS_DATA) {
-         */
+                errno = EIO;
-        if ((decoded = patty_kiss_tnc_decode(tnc, tnc->frame, &framelen, port)) < 0) {
-            patty_kiss_tnc_drop(tnc);
 
-            goto error_io;
+                goto error_io;
+            }
+
+            *port = PATTY_KISS_COMMAND_PORT(c);
+
+            flags |= KISS_COMMAND;
+
+            continue;
         }
 
-        /*
+        if (!(flags & KISS_ESCAPE)) {
-         * If we can no longer buffer nor decode any data, then drop the whole
+            if (c == PATTY_KISS_FESC) {
-         * frame.
+                flags |= KISS_ESCAPE;
-         */
+
-        if (decoded == 0 && buffered == 0) {
+                continue;
-            patty_kiss_tnc_drop(tnc);
+            }
+        } else {
+            switch (c) {
+                case PATTY_KISS_TFEND:
+                    ((uint8_t *)tnc->frame)[b++] = PATTY_KISS_FEND;
+                    flags &= ~KISS_ESCAPE;
+                    continue;
+
+                case PATTY_KISS_TFESC:
+                    ((uint8_t *)tnc->frame)[b++] = PATTY_KISS_FESC;
+                    flags &= ~KISS_ESCAPE;
+                    continue;
+
+                default:
+                    errno = EIO;
+
+                    goto error_io;
+            }
         }
-    } while (decoded == 0);
 
-    /*
+        ((uint8_t *)tnc->frame)[b++] = c;
-     * Flush the buffer up to the point of the next frame start.
+    }
-     */
-    patty_kiss_tnc_flush(tnc, decoded);
 
-    *frame = tnc->frame;
+done:
+    if (flags & KISS_FRAME) {
+        tnc_drop(tnc);
 
-    return framelen;
+        *frame = NULL;
+    } else {
+        tnc_flush(tnc, i);
+
+        *frame = tnc->frame;
+    }
+
+    return (ssize_t)b;
 
 error_io:
     return -1;
@@ -286,7 +224,10 @@ static inline ssize_t write_command(int fd, int command, int port) {
 static uint8_t escape_fend[2] = { PATTY_KISS_FESC, PATTY_KISS_TFEND };
 static uint8_t escape_fesc[2] = { PATTY_KISS_FESC, PATTY_KISS_TFESC };
 
-ssize_t patty_kiss_tnc_send(patty_kiss_tnc *tnc, const void *buf, size_t len, int port) {
+ssize_t patty_kiss_tnc_send(patty_kiss_tnc *tnc,
+                            const void *buf,
+                            size_t len,
+                            int port) {
     size_t i, start = 0, end = 0;
 
     if (write_byte(tnc->fd, PATTY_KISS_FEND) < 0) {