MINOR: ncbuf: refactor ncb_advance()

First adjusted some typos in comments inside the function. Second,
change the naming of some variable to reduce confusion.

A special case has been inserted when advance is done inside a GAP block
and this block is the last of the buffer. In this case, the whole buffer
will be emptied, equivalent to a ncb_init() operation.

diff --git a/include/haproxy/ncbuf.h b/include/haproxy/ncbuf.h
index 68bdb954975744cd9a28613d408ba969680a5aa1..b16e10a3bd9fc50b8298c63e7362dc9ed60bfa32 100644 (file)
--- a/include/haproxy/ncbuf.h
+++ b/include/haproxy/ncbuf.h
@@ -20,6 +20,6 @@ ncb_sz_t ncb_data(const struct ncbuf *buf, ncb_sz_t offset);
 
 enum ncb_ret ncb_add(struct ncbuf *buf, ncb_sz_t off,
                      const char *data, ncb_sz_t len, enum ncb_add_mode mode);
-enum ncb_ret ncb_advance(struct ncbuf *buf, ncb_sz_t off);
+enum ncb_ret ncb_advance(struct ncbuf *buf, ncb_sz_t adv);
 
 #endif /* _HAPROXY_NCBUF_H */

diff --git a/src/ncbuf.c b/src/ncbuf.c
index 91c070aee0e9b6cb10813de99278555b293e7afd..d0ced6d217d90e90bdb46f7149125c43e23de5a8 100644 (file)
--- a/src/ncbuf.c
+++ b/src/ncbuf.c
@@ -600,102 +600,97 @@ enum ncb_ret ncb_add(struct ncbuf *buf, ncb_sz_t off,
        return NCB_RET_OK;
 }
 
-/* Advance the head of <buf> to the offset <off>. Data at the start of buffer
+/* Advance the head of <buf> to the offset <adv>. Data at the start of buffer
  * will be lost while some space will be formed at the end to be able to insert
  * new data.
  *
  * Returns NCB_RET_OK on success.
  */
-enum ncb_ret ncb_advance(struct ncbuf *buf, ncb_sz_t off)
+enum ncb_ret ncb_advance(struct ncbuf *buf, ncb_sz_t adv)
 {
-       struct ncb_blk blk, last;
+       struct ncb_blk start, last;
        ncb_sz_t off_blk;
        ncb_sz_t first_data_sz;
 
-       BUG_ON_HOT(off > ncb_size(buf));
-       if (!off)
+       BUG_ON_HOT(adv > ncb_size(buf));
+       if (!adv)
                return NCB_RET_OK;
 
-       /* Special case if off is full size. This is equivalent to a reset. */
-       if (off == ncb_size(buf)) {
+       /* Special case if adv is full size. This is equivalent to a reset. */
+       if (adv == ncb_size(buf)) {
                ncb_init(buf, buf->head);
                return NCB_RET_OK;
        }
 
-       last = blk = ncb_blk_find(buf, off);
+       start = ncb_blk_find(buf, adv);
+
+       /* Special case if advance until the last block which is a GAP. The
+        * buffer will be left empty and is thus equivalent to a reset.
+        */
+       if (ncb_blk_is_last(buf, start) && (start.flag & NCB_BK_F_GAP)) {
+               ncb_sz_t new_head = buf->head + adv;
+               if (new_head >= buf->size)
+                       new_head -= buf->size;
+
+               ncb_init(buf, new_head);
+               return NCB_RET_OK;
+       }
+
+       last = start;
        while (!ncb_blk_is_last(buf, last))
                last = ncb_blk_next(buf, last);
 
-       off_blk = ncb_blk_off(blk, off);
+       off_blk = ncb_blk_off(start, adv);
 
-       /* If new head points in a GAP, the GAP size must be big enough. */
-       if (blk.flag & NCB_BK_F_GAP) {
-               if (blk.sz == off_blk) {
-                       /* GAP si completely removed. */
-                       first_data_sz = blk.sz_data;
+       if (start.flag & NCB_BK_F_GAP) {
+               /* If advance in a GAP, its new size must be big enough. */
+               if (start.sz == off_blk) {
+                       /* GAP removed. Buffer will start with following DATA block. */
+                       first_data_sz = start.sz_data;
                }
-               else if (!ncb_blk_is_last(buf, blk) &&
-                        blk.sz - off_blk < NCB_GAP_MIN_SZ) {
+               else if (start.sz - off_blk < NCB_GAP_MIN_SZ) {
                        return NCB_RET_GAP_SIZE;
                }
                else {
-                       /* A GAP will be present at the front. */
+                       /* Buffer will start with this GAP block. */
                        first_data_sz = 0;
                }
        }
        else {
-               /* If off_blk less than blk.sz, the data block will becomes the
+               /* If off_blk less than start.sz, the data block will becomes the
                 * first block. If equal, the data block is completely removed
                 * and thus the following GAP will be the first block.
                 */
-               first_data_sz = blk.sz - off_blk;
+               first_data_sz = start.sz - off_blk;
        }
 
-       /* Insert a new GAP if :
-        * - last block is DATA
-        * - last block is GAP and but is not the same as blk
-        *
-        * In the the of last block is a GAP and is the same as blk, it means
-        * that a GAP will be formed to recover the whole buffer content.
-        */
-       if (last.flag & NCB_BK_F_GAP && !ncb_blk_is_last(buf, blk)) {
-               /* last block is a GAP : extends it unless this is a reduced
-                * gap and the new gap size is still not big enough.
-                */
-               if (!(last.flag & NCB_BK_F_FIN) || last.sz + off >= NCB_GAP_MIN_SZ) {
+       if (last.flag & NCB_BK_F_GAP) {
+               /* Extend last GAP unless this is a reduced gap. */
+               if (!(last.flag & NCB_BK_F_FIN) || last.sz + adv >= NCB_GAP_MIN_SZ) {
                        /* use .st instead of .sz_ptr which can be NULL if reduced gap */
-                       ncb_write_off(buf, last.st, last.sz + off);
+                       ncb_write_off(buf, last.st, last.sz + adv);
                        ncb_write_off(buf, ncb_peek(buf, last.off + NCB_GAP_SZ_DATA_OFF), 0);
                }
        }
-       else if (!(last.flag & NCB_BK_F_GAP)) {
-               /* last block DATA : insert a new gap after the deleted data.
-                * If the gap is not big enough, it will be a reduced gap.
-                */
-               if (off >= NCB_GAP_MIN_SZ) {
-                       ncb_write_off(buf, ncb_peek(buf, last.off + last.sz + NCB_GAP_SZ_OFF), off);
+       else {
+               /* Insert a GAP after the last DATA block. */
+               if (adv >= NCB_GAP_MIN_SZ) {
+                       ncb_write_off(buf, ncb_peek(buf, last.off + last.sz + NCB_GAP_SZ_OFF), adv);
                        ncb_write_off(buf, ncb_peek(buf, last.off + last.sz + NCB_GAP_SZ_DATA_OFF), 0);
                }
        }
 
-       /* Advance head and update the buffer reserved header which contains
-        * the first data block size.
-        */
-       buf->head += off;
+       /* Advance head and update reserved header with new first data size. */
+       buf->head += adv;
        if (buf->head >= buf->size)
                buf->head -= buf->size;
        ncb_write_off(buf, ncb_reserved(buf), first_data_sz);
 
-       /* Update the first block GAP size if needed. */
-       if (blk.flag & NCB_BK_F_GAP && !first_data_sz) {
-               /* If first block GAP is also last one, cover whole buf. */
-               if (ncb_blk_is_last(buf, blk))
-                       ncb_write_off(buf, ncb_head(buf), ncb_size(buf));
-               else
-                       ncb_write_off(buf, ncb_head(buf), blk.sz - off_blk);
-
+       /* If advance in a GAP, reduce its size. */
+       if (start.flag & NCB_BK_F_GAP && !first_data_sz) {
+               ncb_write_off(buf, ncb_head(buf), start.sz - off_blk);
                /* Recopy the block sz_data at the new position. */
-               ncb_write_off(buf, ncb_peek(buf, NCB_GAP_SZ_DATA_OFF), blk.sz_data);
+               ncb_write_off(buf, ncb_peek(buf, NCB_GAP_SZ_DATA_OFF), start.sz_data);
        }
 
        return NCB_RET_OK;