ccan
ccan copied to clipboard
Bitmap tal support
My 1st ccan patch - be gentle.
Hi! Thanks, this is great... @dgibson
The main problem is we don't want ccan/bitmap to depend on tal; it's a big thing to pull in. But that just means we should have a bitmap/tal module.
Two other nitpicks:
- We should also keep nomenclature in spec: tal uses 'z' for zero-fill, and bitmap uses 'zero' and 'fill' for 0 and 1 resp.
- The "return the new allocation" is an antipattern, it's better to make bitmap_tal_resize() take the pointer to the pointer. In fact, tal_resize() does not zero the pointer on failure, so your code is actually incorrect here.
I've pushed a proposal on top of your branch, see what you think?
That looks great, I think you should be a co-author :-) :+1:
Follow-up questions:
-
Naming - Well, so what do to when the conventional naming differs between the two?
-
Let's say (purely hypothetically!) that I have a new module in mind that uses bitmaps all over, including creating them dynamically. Is it ok to depend on
tal
then? I really hate usingmalloc
, I think something liketal
should be inlibc
. In factglibc
has obstacks which are poor cousins.
Ian Zimmerman [email protected] writes:
That looks great, I think you should be a co-author :-) :+1:
Follow-up questions:
- Naming - Well, so what do to when the conventional naming differs between the two?
Naming is hard :) I'll leave this to David...
- Let's say (purely hypothetically!) that I have a new module in mind that uses bitmaps all over, including creating them dynamically. Is it ok to depend on
tal
then? I really hate usingmalloc
, I think something liketal
should be inlibc
. In factglibc
has obstacks which are poor cousins.
Yes, or just depend on bitmap/tal now and you'll get it by default.
I like tal, but I try not to depend on it gratuitously.
Cheers, Rusty.
Why does the build fail?
On Thu, Aug 24, 2017 at 12:26:28AM +0000, Rusty Russell wrote:
Ian Zimmerman [email protected] writes:
That looks great, I think you should be a co-author :-) :+1:
Follow-up questions:
- Naming - Well, so what do to when the conventional naming differs between the two?
Naming is hard :) I'll leave this to David...
Heh, thanks.
I'd say this is primarily a bitmap extension using tal, rather than a tal extension using bitmap. So, I think bitmap naming takes priority.
So I'd suggest:
bitmap_tal() bitmap_tal0() bitmap_tal1() bitmap_tal_resize0() bitmap_tal_resize1()
- Let's say (purely hypothetically!) that I have a new module in mind that uses bitmaps all over, including creating them dynamically. Is it ok to depend on
tal
then? I really hate usingmalloc
, I think something liketal
should be inlibc
. In factglibc
has obstacks which are poor cousins.Yes, or just depend on bitmap/tal now and you'll get it by default.
I like tal, but I try not to depend on it gratuitously.
To expand on this: if you think tal simplifies your module enough to be worth it, go ahead and rely on it - it's your module. Just be aware that there's a tradeoff in that it makes it harder to use your module in a program (or another module) that's not tal based.
Whether it's worth it tends to depend on the complexity of your module, and how widely its applicable. e.g. I didn't want bitmap to depend on tal because it potentially has very wide applicability, and it's simple enough that tal() doesn't make things very much easier. On the other hand my rfc822 module does depend on tal, because it could potentially have many bits of allocated cached data which tal makes much easier to manage. Or it would, if I'd ever had time to get that module beyond the barest bones.
-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT the other | way around! http://www.ozlabs.org/~dgibson