Re: [PATCH 1/4] stringbuf: A string buffer implementation

From: Matthew Wilcox
Date: Tue Oct 23 2007 - 22:30:48 EST


On Tue, Oct 23, 2007 at 04:43:53PM -0700, Linus Torvalds wrote:
> On Tue, 23 Oct 2007, Matthew Wilcox wrote:
> > This is a generic string buffer which can also be used for non-printk
> > purposes. There is no sb_scanf implementation yet as I haven't identified
> > a user for it.
>
> Hmm. Have you looked at the git "strbuf" code?

I hadn't. It's quite spookily similar in some ways. There's a lot of
things you've added because you presumably have use for them in git that
could easily be added once we had users for them in the kernel. There's
a few things stringbufs do that strbufs don't need to because they're in
userspace.

I can easily change the API at this point, so if renaming things will
make you happier, just say so.

> And stuff like "sb_string()" are just evil. The string is there. Just call
> it something better than "s", and let people just use "sb->buf" or
> something. Why make the interface harder to use than necessary?

It's a matter of taste ... some people prefer to use accessors for
everything, other people prefer to expose the underlying structure
directly. I have to confess to being influenced by Java in 1998-99,
so possibly I tend to go for accessors too often. Or possibly it's
just my experience of restructuring things in Linux where diffs would
be smaller if we only had used accessors for something. In any case,
I don't care to fight about this; if you want it to be called buf,
then buf it shall be called, and I'll get rid of sb_string.

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/