Re: [PATCH 2/2] strbuf: allow to use preallocated memory

On 05/30/2016 02:52 PM, Matthieu Moy wrote:

I feel bad bikeshedding about names, especially since you took some of the original names from my RFC. But names are very important, so I think it's worth considering whether the current names could be improved upon.

When reading this patch series, I found I had trouble remembering whether "preallocated" meant "preallocated and movable" or "preallocated and immovable". So maybe we should brainstorm alternatives to "preallocated" and "fixed". For example,

* "growable"/"fixed"? Seems OK, though all strbufs are growable at least
to the size of their initial allocation, so maybe "growable" is misleading.

* "movable"/"fixed"? This maybe better captures the essence of the
distinction. I'll use those names below for concreteness, without
claiming that they are the best.

Hmmm....

I think the best way to answer these questions is to think about use cases and make them as easy/consistent as possible.

I expect that a very common use of strbuf_wrap_fixed() will be to wrap a stack-allocated string, like

    char pathbuf[PATH_MAX];
    struct strbuf path;

    strbuf_wrap_fixed(&path, pathbuf, 0, sizeof(pathbuf));

In this use case, it would be a shame if `path` had to be initialized to STRBUF_INIT just because `strbuf_wrap_fixed()` calls `strbuf_release()` internally.

But maybe we could make this use case easier still. If there were a macro

    #define STRBUF_FIXED_WRAPPER(sb, buf, len) { STRBUF_FIXED_MEMORY,
sizeof(buf), (len), (buf) }

then we could write

    char pathbuf[PATH_MAX];
    struct strbuf path = STRBUF_FIXED_WRAPPER(pathbuf, 0);

I think that would be pretty usable. One would have to be careful only to wrap arrays and not `char *` pointers, because `sizeof` wouldn't work on the latter. The BARF_UNLESS_AN_ARRAY macro could be used here to add a little safety.

(It would be even nicer if we could write

    struct strbuf path = STRBUF_FIXED(PATH_MAX);

and it would initialize both path and a pathbuf variable for it to wrap, but I don't think there is a way to implement such a macro. So I think the only way to squeeze this onto one line would be to make it look like

    DEFINE_FIXED_STRBUF(path, PATH_MAX);

But that looks awful, so I think the two-line version above is preferable.)

Similarly, there could be a macro

    #define STRBUF_MOVABLE_WRAPPER(sb, buf, len) { 0, sizeof(buf),
(len), (buf) }

If you provide macro forms like these for initializing strbufs, then I agree with Matthieu that the analogous functional forms should probably call strbuf_release() before wrapping the array. The functions might be named more like `strbuf_attach()` to emphasize their similarity to that existing function. Maybe

    strbuf_attach_fixed(struct strbuf *sb, void *s, size_t len, size_t
alloc);
    strbuf_attach_movable(struct strbuf *sb, void *s, size_t len, size_t
alloc);

Michael