Bitmap tal support

[ghost]

My 1st ccan patch - be gentle.

[rustyrussell]

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:

1. We should also keep nomenclature in spec: tal uses 'z' for zero-fill, and bitmap uses 'zero' and 'fill' for 0 and 1 resp.
2. 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?

[ghost]

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?

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.

[rustyrussell]

Ian Zimmerman <notifications@github.com> 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.

[ghost]

Why does the build fail?

[dgibson]

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