-
Notifications
You must be signed in to change notification settings - Fork 206
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bitmap tal support #60
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Martin Milata <[email protected]>
Signed-off-by: Rusty Russell <[email protected]>
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:
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 :-) 👍 Follow-up questions:
|
Ian Zimmerman <[email protected]> writes:
That looks great, I think you should be a co-author :-) 👍
Follow-up questions:
1. Naming - Well, so what do to when the conventional naming differs between the two?
Naming is hard :) I'll leave this to David...
2. 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 using `malloc`, I think something like `tal` should be in `libc`. In fact `glibc` 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 ***@***.***> writes:
> That looks great, I think you should be a co-author :-) :+1:
>
> Follow-up questions:
>
> 1. 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()
> 2. 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 using `malloc`, I think something like `tal` should be in `libc`. In fact `glibc` 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
|
My 1st ccan patch - be gentle.