Pierre Habouzit wrote:
On Tue, Sep 04, 2007 at 11:11:45AM +0000, Johannes Schindelin wrote:
Hi,
On Tue, 4 Sep 2007, Pierre Habouzit wrote:
- IMHO the same goes for strbuf_free() instead of strbuf_wipe(), and
Well, I don't like strbuf_free, because it's opposed to
strbuf_alloc/new whatever, that would be functions that create and
release a struct strbuf * (meaning allocating the struct shell as well).
Here you only free the internal buffer, not the shell, so _free() would
be a very bad name.
In my own coding rules, I have _new/_delete and _init/_wipe functions,
the former acts on pointers, the latter on the structs. Hence the
naming. Though, looking at git's code, it seems that the usual name for
this operation is _release. So would you go for strbuf_release ?
release is indeed better. To me, "wipe" means zeroing out (possibly after
an optional number of passes writing trash to the memory area) rather than
actually freeing any memory.
- it would be nice to split this patch into
- the API change (with _minimal_ changes to anything outside of
strbuf.[ch]), and
- the cleanups in the rest of the code.
I'll try to do that, but it's quite hard to achieve knowing that in
many places of the current state of master, there are embeded NUL's
(accounted in ->len). I'll try to see what I can split, but it's
unlikely I'll be able to have an intermediate _working_ state (I mean
that would pass the testsuite) with the new API/semantics _and_ without
touching a lot less of the rest of the code.
Always having a compilable/operational tree is pretty nice primarily due
to bisect. If the code turns from working to non-working in between,
make sure to jot it down in the commit message.
It might even be better to add the embedded NUL handling code in the
first patch, modify the callers in the second, and remove the embedded
NUL handling in a third.
--
Andreas Ericsson andreas.ericsson@xxxxxx
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
