tal_stack
authorDelio Brignoli <brignoli.delio@gmail.com>
Sun, 12 Apr 2015 13:27:25 +0000 (15:27 +0200)
committerRusty Russell <rusty@rustcorp.com.au>
Mon, 13 Apr 2015 02:11:00 +0000 (11:41 +0930)
commit73930140fa25d3834570fcd906a01e199cfe17ac
treebc7a4990d38453710c32c71ee5e1046d50e7d473
parentbdb8d75154d7aefe01788e5dd5adb11e3c943c11
tal_stack

Hi Rusty,

Thanks for reviewing the patch. V2 is attached, see my comments below.

> On 31 Mar 2015, at 02:36, Rusty Russell <rusty@rustcorp.com.au> wrote:
>
> Delio Brignoli <brignoli.delio@gmail.com> writes:
>> Hi All,
>>
>> tal_stack implements a (trivial) stack of tal contexts. Would this be a worthy addition to CCAN? (not necessarily in its current form).

[…]

>        This is cute; I’ve seen similar used in Samba.  It's

Indeed, it was inspired by talloc_stack.h ;-)

[…]

> You are missing a _info file: I would create that, and put your example
> in an Example: section there.

I moved the module and tests under can/tal/stack and added a LICENSE and _info.

> Other random advice:
> 1) You should also document the tal_newframe function (particularly note
>   that you're expected to tal_free the result, and that it will free
>   any future unfreed frames).  And note that it’s not threadsafe.

Done.

> 2) You probably want tal_newframe to be a macro, and hand file and line
>   thought to the tal_alloc_ call.  That makes debugging nicer when
>   you iterate the tree.

Done. The macro is calling a tal_newframe_() function because I’d rather not make the module’s stack variable ‘public’.

> 3) Consider whether you want to declare a dummy type 'struct tal_stack'.
>   Probably pretty unnecessary since it’s quite clear.

Skipped this one. We can declare it later if we change our minds.

Thanks

Delio

From c2ceb9258d97b0dcb72e7b6986cfd2bd394b254e Mon Sep 17 00:00:00 2001
From: Delio Brignoli <dbrignoli@audioscience.com>
Date: Sun, 15 Mar 2015 13:26:40 +0100
Subject: [PATCH] tal_stack: new module - V2

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Makefile-ccan
ccan/tal/stack/LICENSE [new symlink]
ccan/tal/stack/_info [new file with mode: 0644]
ccan/tal/stack/stack.c [new file with mode: 0644]
ccan/tal/stack/stack.h [new file with mode: 0644]
ccan/tal/stack/test/run-stack.c [new file with mode: 0644]