Re: [PATCH] New strbuf APIs: splice and embed.

Pierre Habouzit [off-list ref] writes:

* strbuf_splice replace a portion of the buffer with another.
* strbuf_embed replace a strbuf buffer with the given one, that should be
  malloc'ed. Then it enforces strbuf's invariants. If alloc > len, then this
  function has negligible cost, else it will perform a realloc, possibly
  with a cost.

"embed" does not sound quite right, does it?  It is a reverse
operation of strbuf_detach() as far as I can tell.

-void strbuf_rtrim(struct strbuf *sb)
-{
+void strbuf_rtrim(struct strbuf *sb) {
 	while (sb->len > 0 && isspace((unsigned char)sb->buf[sb->len - 1]))
 		sb->len--;
 	sb->buf[sb->len] = '\0';

This is changing the style in the wrong direction, isn't it?  We
start our functions like this:

	type name(proto)
        {
        	...

+void strbuf_splice(struct strbuf *sb, size_t pos, size_t len,
+				   const void *data, size_t dlen)
+{
+	if (pos + len < pos)
+		die("you want to splice outside from the buffer");

That is a funny error message for an integer wrap-around check.

+	if (pos > sb->len)
+		pos = sb->len;

Shouldn't this be flagged as a programming error?

+	if (pos + len > sb->len)
+		len = sb->len - pos;

Likewise.

By the way, this is the kind of situation I wish everybody wrote
their comparison in textual order.  I had to draw a picture like
this to see what was going on.

        sb->buf
        xxxxxxxxxxxzzzzzxxxxxxxxxxx\0
        ^                         ^
        0                   sb->len
                   ^    ^               ^    ^        
                   pos  pos+len         pos  pos+len  
                   v                    v
                   yyyyyyyyyy           yyyyyyyyyy    
                             ^                    ^   
                             dlen                 dlen
                   -------------        --------------
                   pos < sb->len        sb->len < pos