Re: [ccan] [PATCH 5/5] altstack: Don't log internal calls in test cases
Very well, I've applied your patches as provided (except I dropped the
trailing underscore from state.rsp_save).
On Thu, Jun 16, 2016 at 7:12 AM David Gibson
wrote:
> On Sun, Jun 12, 2016 at 02:10:18AM +0000, Dan Good wrote:
> > Hairy macros? From the author of the cppmagic module, I shall take that
> as
> > a compliment.
>
> Touché.
>
> That said, cppmagic is using hairy magic to do some things that are,
> as far as I know, impossible by any other method. I don't think
> there's a similar justification here.
>
> > The purpose of the setcall macro and related checks is ensure the
> > correctness of the error path, i.e. if setrlimit ran before a failure, it
> > must run again to undo the first; if mmap ran before a failure, munmap
> must
> > run after. I find it very reassuring to know these tests exist and pass.
> > Do you see no value in that?
>
> So, there's certainly value in checking the error paths, but doing it
> by checking what calls the implementation makes is not a great way of
> doing it. It's pretty subject to both false positives and false
> negatives.
>
> What I'd suggest instead is actually checking the behaviour in
> question. For example, if you want to check that the rlimit is
> restored properly I'd suggest:
> 1. Call getrlimit()
> 2. Call altstack() in a way rigged to fail
> 3. Call getrlimit() again
> 4. Compare results from (1) and (3)
>
> > I don't really see a need to optimize for changing altstack's
> > implementation. It's less than a hundred lines of code, and only ten
> tests
> > using setcall. Can you tell me why you think it's important?
>
> So, the checking of specific calls makes the tests very dependent on
> altstack's implementation, and the framework of macros used to do it
> makes it difficult to change one test without changing them all.
>
> Together, that makes it almost impossible to change anything
> significant about the altstack implementation without having to
> significantly rewrite the tests. And if the tests are significantly
> rewritten, it's hard to be confident that they still check the things
> they used to.
>
> Which undermines the whole value of a testsuite in allowing you to
> modify the implementation while being reasonably confident you haven't
> changed the desired behaviour.
>
> This is not theoretical; I have a couple of changes in mind for which
> the primary obstacle is adjusting the testsuite to match (switching to
> ccan/coroutine to avoid the x86_64 specific code, and using mprotect()
> instead of MAP_GROWSDOWN+setrlimit()).
>
> > On Fri, Jun 3, 2016 at 4:40 AM David Gibson >
> > wrote:
> >
> > > altstack/test/run.c uses some hairy macros to intercept the standard
> > > library functions that altstack uses. This has two purposes: 1) to
> > > conditionally cause those functions to fail, and thereby test
> altstack's
> > > error paths, and 2) log which of the library functions was called in
> each
> > > testcase.
> > >
> > > The second function isn't actually useful - for the purposes of
> testing the
> > > module, we want to check the actual behaviour, not what calls it made
> in
> > > what order to accomplish it. Explicitly checking the calls makes it
> much
> > > harder to change altstack's implementation without breaking the tests.
> > >
> > > Signed-off-by: David Gibson
> > > ---
> > > ccan/altstack/test/run.c | 73
> > > ++--
> > > 1 file changed, 21 insertions(+), 52 deletions(-)
> > >
> > > diff --git a/ccan/altstack/test/run.c b/ccan/altstack/test/run.c
> > > index e109ccb..091d1f5 100644
> > > --- a/ccan/altstack/test/run.c
> > > +++ b/ccan/altstack/test/run.c
> > > @@ -20,18 +20,17 @@ enum {
> > > sigaction_ = 1<<4,
> > > munmap_ = 1<<5,
> > > };
> > > -int fail, call1, call2;
> > > +int fail;
> > > char *m_;
> > > rlim_t msz_;
> > > #define e(x) (900+(x))
> > > #define seterr(x) (errno = e(x))
> > > -#define setcall(x) ((call1 |= !errno ? (x) : 0), (call2 |= errno ||
> > > state.out ? (x) : 0))
> > > -#define getrlimit(...) (fail&getrlimit_?
> > > (seterr(getrlimit_), -1) : (setcall(getrlimit_),
> > > getrlimit(__VA_ARGS__)))
> > > -#define mmap(...) (fail&mmap_ ?
> (seterr(mmap_),
> &
Re: [ccan] [PATCH 4/5] altstack: Don't use 0 pointer literals
Thank you, both. I'll return to the fold and use NULL, as seems right and proper. On Thu, Jun 16, 2016 at 7:12 AM David Gibson wrote: > On Tue, Jun 14, 2016 at 01:45:10PM +0930, Paul 'Rusty' Russell wrote: > > Dan Good writes: > > > Hi David, > > > > > > I'm back home in front of a real keyboard, and want to follow up with > you > > > about your patches. I must admit that my usual practice is to use > NULL, > > > and not 0. Around the time I wrote altstack I had recently read Jen > > > Gustedt's book, Modern C. In it, he makes the point that NULL hides > more > > > than it clarifies (see section 11.7). > > > > (I don't really care: your code, your style). But... > > > > He is so horrifically wrong, it's amazing. > > > > Section 7.17 is loosened from ANSI C which said NULL was 0, presumably > > to *allow* compilers to avoid the varargs issue. A compiler *could* do > > insane shit to make that problem even worse, but you would never use > > such a compiler. There are other legal-but-insane things a compiler can > > do, too, and the answer is the same; real code won't work, nobody else > > cares. > > > > OTOH, using 0 in place of NULL makes for much more potential type order > > confusion. Not to mention confusing every damn C programmer on the > > planet. > > Yeah. I can kind of see the hint of a good idea in those articles, > but on balance they're really not convincing. Basically, as Rusty > says, matching the conventions of the huge bulk of existing C code > outweighs the value of working with a compiler/library that has gone > out of its way to implement this stupidly. > > > Note, I also assume NULL is all zero-bits, > > I try to avoid that one, although I can't be sure I always have - I'm > not sure, but I think one of the s390 variants might break this. > > > that size_t can hold a > > ptrdiff_t, > > Dunno if I've assumed that much. > > > and that a pointer to a function which takes a type * > > argument can be cast and called as a function which takes a void *. > > Yeah, I've used that. > > > If someone ports CCAN to a platform where those are not true, I'd be > > fascinated... > > -- > 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 > ___ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan
Re: [ccan] [PATCH 5/5] altstack: Don't log internal calls in test cases
Hairy macros? From the author of the cppmagic module, I shall take that as
a compliment.
The purpose of the setcall macro and related checks is ensure the
correctness of the error path, i.e. if setrlimit ran before a failure, it
must run again to undo the first; if mmap ran before a failure, munmap must
run after. I find it very reassuring to know these tests exist and pass.
Do you see no value in that?
I don't really see a need to optimize for changing altstack's
implementation. It's less than a hundred lines of code, and only ten tests
using setcall. Can you tell me why you think it's important?
On Fri, Jun 3, 2016 at 4:40 AM David Gibson
wrote:
> altstack/test/run.c uses some hairy macros to intercept the standard
> library functions that altstack uses. This has two purposes: 1) to
> conditionally cause those functions to fail, and thereby test altstack's
> error paths, and 2) log which of the library functions was called in each
> testcase.
>
> The second function isn't actually useful - for the purposes of testing the
> module, we want to check the actual behaviour, not what calls it made in
> what order to accomplish it. Explicitly checking the calls makes it much
> harder to change altstack's implementation without breaking the tests.
>
> Signed-off-by: David Gibson
> ---
> ccan/altstack/test/run.c | 73
> ++--
> 1 file changed, 21 insertions(+), 52 deletions(-)
>
> diff --git a/ccan/altstack/test/run.c b/ccan/altstack/test/run.c
> index e109ccb..091d1f5 100644
> --- a/ccan/altstack/test/run.c
> +++ b/ccan/altstack/test/run.c
> @@ -20,18 +20,17 @@ enum {
> sigaction_ = 1<<4,
> munmap_ = 1<<5,
> };
> -int fail, call1, call2;
> +int fail;
> char *m_;
> rlim_t msz_;
> #define e(x) (900+(x))
> #define seterr(x) (errno = e(x))
> -#define setcall(x) ((call1 |= !errno ? (x) : 0), (call2 |= errno ||
> state.out ? (x) : 0))
> -#define getrlimit(...) (fail&getrlimit_?
> (seterr(getrlimit_), -1) : (setcall(getrlimit_),
> getrlimit(__VA_ARGS__)))
> -#define mmap(...) (fail&mmap_ ? (seterr(mmap_),
> (void *)-1) : (setcall(mmap_), mmap(__VA_ARGS__)))
> -#define munmap(a, b) (fail&munmap_ ?
> (seterr(munmap_), -1) : (setcall(munmap_),
> munmap(m_=(a), msz_=(b
> -#define setrlimit(...) (fail&setrlimit_?
> (seterr(setrlimit_), -1) : (setcall(setrlimit_),
> setrlimit(__VA_ARGS__)))
> -#define sigaltstack(...) (fail&sigaltstack_ ?
> (seterr(sigaltstack_),-1) : (setcall(sigaltstack_),
> sigaltstack(__VA_ARGS__)))
> -#define sigaction(...) (fail&sigaction_?
> (seterr(sigaction_), -1) : (setcall(sigaction_),
> sigaction(__VA_ARGS__)))
> +#define getrlimit(...) (fail&getrlimit_?
> (seterr(getrlimit_), -1) : getrlimit(__VA_ARGS__))
> +#define mmap(...) (fail&mmap_ ? (seterr(mmap_),
> (void *)-1) : mmap(__VA_ARGS__))
> +#define munmap(a, b) (fail&munmap_ ?
> (seterr(munmap_), -1) : munmap(m_=(a), msz_=(b)))
> +#define setrlimit(...) (fail&setrlimit_?
> (seterr(setrlimit_), -1) : setrlimit(__VA_ARGS__))
> +#define sigaltstack(...) (fail&sigaltstack_ ?
> (seterr(sigaltstack_),-1) : sigaltstack(__VA_ARGS__))
> +#define sigaction(...) (fail&sigaction_?
> (seterr(sigaction_), -1) : sigaction(__VA_ARGS__))
>
> #define KiB (1024UL)
> #define MiB (KiB*KiB)
> @@ -58,74 +57,48 @@ static void *wrap(void *i)
> return wrap;
> }
>
> -#define chkfail(x, y, z, c1, c2) \
> +#define chkfail(x, y, z) \
> do {\
> - call1 = 0; \
> - call2 = 0; \
> errno = 0; \
> ok1((fail = x) && (y)); \
> ok1(errno == (z)); \
> - ok1(call1 == (c1)); \
> - ok1(call2 == (c2)); \
> } while (0);
>
> -#define chkok(y, z, c1, c2)\
> +#define chkok(y, z)\
> do {\
> - call1 = 0; \
> - call2 = 0; \
> errno = 0; \
> fail = 0;
Re: [ccan] [PATCH 4/5] altstack: Don't use 0 pointer literals
Hi David,
I'm back home in front of a real keyboard, and want to follow up with you
about your patches. I must admit that my usual practice is to use NULL,
and not 0. Around the time I wrote altstack I had recently read Jen
Gustedt's book, Modern C. In it, he makes the point that NULL hides more
than it clarifies (see section 11.7). While I find his argument a little
weak when considering a gcc environment where I know that NULL is defined
as ((void *)0), it seems sound when the target tool chain isn't known in
advance, as with a CCAN module. Here's a link to the book and also a blog
article from Jens about NULL. I'm very curious to know if you (or any
other CCAN contributors) find his arguments at all compelling. Thanks.
-Dan
http://icube-icps.unistra.fr/img_auth.php/d/db/ModernC.pdf
https://gustedt.wordpress.com/2010/11/07/dont-use-null/
On Fri, Jun 3, 2016 at 4:40 AM David Gibson
wrote:
> In a number of places the altstack module uses a literal '0' for pointer
> values. That's correct C, but doesn't make it obvious on a quick read
> whether values are integers or pointers. This patch changes those cases
> to use the NULL define instead.
>
> Signed-off-by: David Gibson
> ---
> ccan/altstack/_info | 4 ++--
> ccan/altstack/altstack.c | 10 +-
> ccan/altstack/altstack.h | 6 +++---
> ccan/altstack/test/run.c | 22 +++---
> 4 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/ccan/altstack/_info b/ccan/altstack/_info
> index 7accf86..b8cf6a8 100644
> --- a/ccan/altstack/_info
> +++ b/ccan/altstack/_info
> @@ -63,8 +63,8 @@
> * void *out;
> *
> * assert(argc == 3);
> - * stk_max = strtol(argv[1], 0, 0) * 1024 * 1024;
> - * vla_sz = strtol(argv[2], 0, 0) * 1024 * 1024;
> + * stk_max = strtol(argv[1], NULL, 0) * 1024 * 1024;
> + * vla_sz = strtol(argv[2], NULL, 0) * 1024 * 1024;
> * assert(stk_max > 0 && vla_sz > 0);
> *
> * snprintf(maps, sizeof(maps), "egrep '\\[stack'
> /proc/%d/maps", getpid());
> diff --git a/ccan/altstack/altstack.c b/ccan/altstack/altstack.c
> index 6dfb9fa..2115740 100644
> --- a/ccan/altstack/altstack.c
> +++ b/ccan/altstack/altstack.c
> @@ -78,10 +78,10 @@ int altstack(rlim_t max, void *(*fn)(void *), void
> *arg, void **out)
>
> state.fn = fn;
> state.arg = arg;
> - state.out = 0;
> + state.out = NULL;
> state.max = max;
> state.ebuf[state.elen = 0] = '\0';
> - if (out) *out = 0;
> + if (out) *out = NULL;
>
> // if the first page below the mapping is in use, we get max-pgsz
> usable bytes
> // add pgsz to max to guarantee at least max usable bytes
> @@ -91,7 +91,7 @@ int altstack(rlim_t max, void *(*fn)(void *), void *arg,
> void **out)
> ok(setrlimit(RLIMIT_STACK, &(struct rlimit) { state.max,
> rl_save.rlim_max }), 1);
> undo++;
>
> - ok(m = mmap(0, max, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_ANONYMOUS|MAP_GROWSDOWN|MAP_NORESERVE, -1, 0), 1);
> + ok(m = mmap(NULL, max, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_ANONYMOUS|MAP_GROWSDOWN|MAP_NORESERVE, -1, 0), 1);
> undo++;
>
> if (setjmp(state.jmp) == 0) {
> @@ -130,9 +130,9 @@ out:
>
> switch (undo) {
> case 4:
> - ok(sigaction(SIGSEGV, &sa_save, 0), 0);
> + ok(sigaction(SIGSEGV, &sa_save, NULL), 0);
> case 3:
> - ok(sigaltstack(&ss_save, 0), 0);
> + ok(sigaltstack(&ss_save, NULL), 0);
> case 2:
> ok(munmap(m, max), 0);
> case 1:
> diff --git a/ccan/altstack/altstack.h b/ccan/altstack/altstack.h
> index 4445a2a..09ffe0b 100644
> --- a/ccan/altstack/altstack.h
> +++ b/ccan/altstack/altstack.h
> @@ -52,7 +52,7 @@
> * static void *wrap(void *i)
> * {
> * dn((unsigned long) i);
> - * return 0;
> + * return NULL;
> * }
> *
> * #define MiB (1024UL*1024UL)
> @@ -60,9 +60,9 @@
> * {
> * unsigned long n;
> * assert(argc == 2);
> - * n = strtoul(argv[1], 0, 0);
> + * n = strtoul(argv[1], NULL, 0);
> *
> - * if (altstack(32*MiB, wrap, (void *) n, 0) != 0)
> + * if (altstack(32*MiB, wrap, (void *) n, NULL) != 0)
> * altstack_perror();
> *
> * printf("%d\n", depth);
> diff --git a/ccan/altstack/test/run.c b/ccan/altstack/test/run.c
> index 61710fd..e109ccb 100644
> --- a/ccan/altstack/test/run.c
> +++ b/ccan/altstack/test/run.c
> @@ -87,43 +87,43 @@ int main(void)
>
> plan_tests(50);
>
> - chkfail(getrlimit_, altstack(8*MiB, wrap, int2ptr(0), 0) ==
> -1, e(getrlimit_),
> + chkfail(getrlimit_, altstack(8*MiB, wrap, int2ptr(0), NULL) ==
> -1, e(getrlimit_),
> 0,
> 0);
>
> - chkfail(setrlimit_, altstack(
Re: [ccan] [PATCH 0/5] altstack: cleanups
David, thank you for improving the code. I'm traveling for the next week with only an ipad. I'd like to ask your thoughts on a topic or two, but typing on this is grueling. I hope to try for a longer reply later. Thanks again. -Dan On Fri, Jun 3, 2016 at 4:40 AM David Gibson wrote: > Dan, > > Here are a bunch of assorted cleanups to the altstack module. If they > seem reasonable to you, please apply. > > David Gibson (5): > altstack: Consolidate thread-local variables > altstack: Restore alternate signal stack state > altstack: Use ptrint instead of bare casts > altstack: Don't use 0 pointer literals > altstack: Don't log internal calls in test cases > > ccan/altstack/_info | 9 -- > ccan/altstack/altstack.c | 78 > +--- > ccan/altstack/altstack.h | 6 ++-- > ccan/altstack/test/run.c | 76 > ++ > 4 files changed, 74 insertions(+), 95 deletions(-) > > -- > 2.5.5 > > ___ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan
Re: [ccan] altstack causing travis-ci failures
David did some remarkable investigation into it about a month ago. The code has always worked on the Red Hat based distros I use, and David patched it to work for Ubuntu 12.04; so it seems unlikely to me that any real person would be affected. I suppose I'll try to talk to the Travis people about the specifics of their environment. On Fri, Mar 25, 2016 at 10:14 AM Andrew Jeffery wrote: > Hey Dan, > > I'm seeing a ccanlint failure for altstack on Travis-CI, e.g[1]: > > ... > tools/ccanlint/ccanlint --deps-fail-ignore -s ccan/aga > tools/ccanlint/ccanlint --deps-fail-ignore -s ccan/agar > agar: Total score: 36/38 > tools/ccanlint/ccanlint --deps-fail-ignore -s ccan/altstack > altstack: Total score: 31/36 FAIL! > make: *** [summary-check-altstack] Error 1 > tools/ccanlint/ccanlint --deps-fail-ignore -s ccan/antithread > aga: Total score: 48/50 > ... > > FWIW I haven't had this happen when running `make check` on my machine. > I ended up pushing the changes I was testing as the failure wasn't > related and wasn't something I could trigger locally (the commit tested > above is currently at the tip of master), but thought it would be > useful to highlight that something strange appears to be happening with > the altstack module, at least on Travis-CI. > > Cheers, > > Andrew > > [1] https://travis-ci.org/amboar/ccan/jobs/118466416#L534 ___ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan
Re: [ccan] [PATCH] altstack: stack alignment and accounting tweaks
I see. Thank you. On Tue, Feb 16, 2016 at 5:04 AM David Gibson wrote: > On Mon, Feb 15, 2016 at 06:29:10PM +0000, Dan Good wrote: > > I thought I understood from our emails at the end of last year that a > > single "polished" commit was desirable. Is that not the same thing as > > taking several very small commits and bundling them into one? > > Often not, in practice. The ultimate goal is reviewability (both > before merging and when looking back at the commit history). But > that's often accomplished different ways depending on the situation. > In particular it tends to be different for initial commits of new code > from changes to existing code. > > For initial commits, it's generally best to elide patches which > correct false steps made during development, or really anything which > doesn't increase the total code size or complexity significantly. > That gives less commits to review, and cleaner code in them to > understand. > > For changes to existing code, however, the important thing is > understanding why the change is desirable and then that the changes > actually accomplish that goal. That tends towards small, single > purpose commits, so it's easy to see what changes belong to what goal > (explained in the commit message). > > Of course, that's a very broad rule of tumb, with plenty of exceptions > and wiggle room. > > > On Mon, Feb 15, 2016 at 5:53 AM David Gibson < > da...@gibson.dropbear.id.au> > > wrote: > > > > > On Wed, Feb 03, 2016 at 11:42:11PM +, Dan Good wrote: > > > > "messy bitser" - what does that mean? That's bad, isn't it? > > > > > > It's... mildly bad. > > > > > > By "bitser" I mean that it contains a number of changes that aren't > > > closely related to each other. In general that makes a patch more > > > difficult to review. How important that is depends on the complexity > > > of the changes and how mature the code being altered is, so it's not > > > really a big deal. > > > > > > The fact that altstack has been breaking Travis builds for a while > > > probably predisposed me to be uncharitable. > > > > > > > Can I help fix the builds? > > > > > > So, I've figured out what the problem is, patch coming shortly. > > > > > > > > > > > On Wed, Feb 3, 2016 at 4:28 PM David Gibson < > da...@gibson.dropbear.id.au > > > > > > > > wrote: > > > > > > > > > On Mon, Feb 01, 2016 at 02:43:16AM +, Dan Good wrote: > > > > > > > > > > > > * add altstack_remn, returns amount of stack remaining > > > > > > * increase mapping by 1 page to handle abutment case > > > > > > * capture rsp earlier > > > > > > * align stack to 16 bytes > > > > > > > > > > > > Signed-off-by: Dan Good > > > > > > > > > > Bit of a messy bitser, but doesn't look it will make anything > > > > > worse. so go ahead and push. > > > > > > > > > > Unfortunately, altstack is breaking travis builds at the moment > (see, > > > > > e.g. https://travis-ci.org/dgibson/ccan/jobs/106661592) - but not > > > > > local builds for me which is making it difficult to debug. > > > > > > > > > > Some investigation showed that the test prog was getting a SIGBUS, > but > > > > > I didn't get further than that. > > > > > > > > > > altstack also breaks "make all" on any non-x86_64 platform > (including > > > > > 32-bit x86) which is pretty horrid. For that I think we need to > > > > > adjust the makefiles to look at rusty's new "ported" stuff from > _info. > > > > > > > > > > > > > > > > --- > > > > > > ccan/altstack/altstack.c | 17 ++--- > > > > > > ccan/altstack/altstack.h | 14 ++ > > > > > > ccan/altstack/test/run.c | 20 +--- > > > > > > 3 files changed, 41 insertions(+), 10 deletions(-) > > > > > > > > > > > > diff --git a/ccan/altstack/altstack.c b/ccan/altstack/altstack.c > > > > > > index 6faf38f..b71c64f 100644 > > > > > > --- a/ccan/altstack/altstack.c > > > > > > +++ b/ccan/altstack/altstack.c > > > > > > @@ -8,6
Re: [ccan] [PATCH 3/4] altstack: Declare memory clobbers
Thank you for the fixes and improvements. The clobber change is the only
that gives me pause. I think the volatile keyword on both is sufficient to
prevent re-ordering. Are you sure we need the memory clobber? Thanks.
-Dan
On Tue, Feb 16, 2016 at 5:04 AM David Gibson
wrote:
> altstack includes a couple of inline asm blocks with x86 push and pop
> instructions. These instructions will access memory (the stack), but
> that's not declared in inline asm statement. We seem to be getting away
> with it, but in theory that could allow the compiler to re-order accesses
> to local variables across the asm block. Since those blocks change the
> location of the stack, that could be very bad.
>
> Adding a "memory" clobber should prevent this (effectively making the asm
> blocks a compiler memory barrier).
>
> Signed-off-by: David Gibson
> ---
> ccan/altstack/altstack.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/ccan/altstack/altstack.c b/ccan/altstack/altstack.c
> index 640344d..6351293 100644
> --- a/ccan/altstack/altstack.c
> +++ b/ccan/altstack/altstack.c
> @@ -108,9 +108,10 @@ int altstack(rlim_t max, void *(*fn)(void *), void
> *arg, void **out)
> "mov %1, %%rsp\n\t"
> "sub $8, %%rsp\n\t"
> "push %%r10"
> - : "=r" (rsp_save_[0]) : "0" (m + max) : "r10");
> + : "=r" (rsp_save_[0]) : "0" (m + max) : "r10",
> "memory");
> out_ = fn_(arg_);
> - asm volatile ("pop %rsp");
> + asm volatile ("pop %%rsp"
> + : : : "memory");
> ret = 0;
> if (out) *out = out_;
> }
> --
> 2.5.0
>
>
___
ccan mailing list
ccan@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/ccan
Re: [ccan] git error on push
Don't I feel silly. Thank you. All better now. On Wed, Feb 10, 2016 at 8:16 PM David Gibson wrote: > On Wed, Feb 10, 2016 at 07:15:02PM +0000, Dan Good wrote: > > Hi, I'm trying to push the stack alignment patch to the repo, but no joy: > > > > 0$ git push origin masterCounting objects: 8, done. > > Compressing objects: 100% (7/7), done. > > Writing objects: 100% (8/8), 1.42 KiB | 0 bytes/s, done. > > Total 8 (delta 5), reused 0 (delta 0) > > remote: W NAME/ccan/altstack/altstack.c ccan dancancode DENIED by > fallthru > > remote: error: hook declined to update refs/heads/master > > To c...@ozlabs.org:ccan > > ! [remote rejected] master -> master (hook declined) > > error: failed to push some refs to 'c...@ozlabs.org:ccan' > > 1$ > > > > Is altstack on lockdown? Thanks. -Dan > > Well, it's certainly not supposed to be. I had a look at the gitolite > configuration and I'm not seeing anything obvious wrong: > > RW NAME/ccan/altstack = dancancode > > Have you double checked that you're using the same ssh key as the one > you sent to us? > > -- > 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 > ___ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan
[ccan] git error on push
Hi, I'm trying to push the stack alignment patch to the repo, but no joy: 0$ git push origin masterCounting objects: 8, done. Compressing objects: 100% (7/7), done. Writing objects: 100% (8/8), 1.42 KiB | 0 bytes/s, done. Total 8 (delta 5), reused 0 (delta 0) remote: W NAME/ccan/altstack/altstack.c ccan dancancode DENIED by fallthru remote: error: hook declined to update refs/heads/master To c...@ozlabs.org:ccan ! [remote rejected] master -> master (hook declined) error: failed to push some refs to 'c...@ozlabs.org:ccan' 1$ Is altstack on lockdown? Thanks. -Dan ___ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan
[ccan] [PATCH] altstack: stack alignment and accounting tweaks
* add altstack_remn, returns amount of stack remaining
* increase mapping by 1 page to handle abutment case
* capture rsp earlier
* align stack to 16 bytes
Signed-off-by: Dan Good
---
ccan/altstack/altstack.c | 17 ++---
ccan/altstack/altstack.h | 14 ++
ccan/altstack/test/run.c | 20 +---
3 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/ccan/altstack/altstack.c b/ccan/altstack/altstack.c
index 6faf38f..b71c64f 100644
--- a/ccan/altstack/altstack.c
+++ b/ccan/altstack/altstack.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
static __thread char ebuf[ALTSTACK_ERR_MAXLEN];
@@ -37,6 +38,11 @@ static void segvjmp(int signum)
}
static __thread void *rsp_save_[2];
+static __thread rlim_t max_;
+
+rlim_t altstack_max(void) {
+ return max_;
+}
static ptrdiff_t rsp_save(unsigned i) {
assert(i < 2);
@@ -57,6 +63,7 @@ static __thread void *arg_, *out_;
int altstack(rlim_t max, void *(*fn)(void *), void *arg, void **out)
{
+ long pgsz = sysconf(_SC_PAGESIZE);
int ret = -1, undo = 0;
char *m;
struct rlimit rl_save;
@@ -69,11 +76,16 @@ int altstack(rlim_t max, void *(*fn)(void *), void *arg,
void **out)
fn_ = fn;
arg_ = arg;
out_ = 0;
+ max_ = max;
ebuf[elen = 0] = '\0';
if (out) *out = 0;
+ // if the first page below the mapping is in use, we get max-pgsz
usable bytes
+ // add pgsz to max to guarantee at least max usable bytes
+ max += pgsz;
+
ok(getrlimit(RLIMIT_STACK, &rl_save), 1);
- ok(setrlimit(RLIMIT_STACK, &(struct rlimit) { max, rl_save.rlim_max }),
1);
+ ok(setrlimit(RLIMIT_STACK, &(struct rlimit) { max_, rl_save.rlim_max
}), 1);
undo++;
ok(m = mmap(0, max, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_ANONYMOUS|MAP_GROWSDOWN|MAP_NORESERVE, -1, 0), 1);
@@ -91,8 +103,7 @@ int altstack(rlim_t max, void *(*fn)(void *), void *arg,
void **out)
ok(sigaction(SIGSEGV, &sa, &sa_save), 1);
undo++;
- asm volatile ("movq %%rsp, %%r10\nmov %0, %%rsp\npush %%r10" :
: "g" (m + max) : "r10");
- rsp_save(0);
+ asm volatile ("movq %%rsp, %%r10\nmov %1, %%rsp\nmov %%rsp,
%0\nsub $8, %%rsp\npush %%r10" : "=g" (rsp_save_[0]) : "g" (m + max) : "r10");
out_ = fn_(arg_);
asm volatile ("pop %rsp");
ret = 0;
diff --git a/ccan/altstack/altstack.h b/ccan/altstack/altstack.h
index 5570e7b..4445a2a 100644
--- a/ccan/altstack/altstack.h
+++ b/ccan/altstack/altstack.h
@@ -104,6 +104,20 @@ char *altstack_geterr(void);
ptrdiff_t altstack_used(void);
/**
+ * altstack_max - return usable stack size
+ *
+ * Returns: max value from altstack() call
+ */
+rlim_t altstack_max(void);
+
+/**
+ * altstack_remn - return amount of stack remaining
+ *
+ * Returns: altstack_max() minus altstack_used()
+ */
+#define altstack_remn() (altstack_max() - altstack_used())
+
+/**
* altstack_rsp_save - set initial rsp value
*
* Capture the current value of rsp for future altstack_used()
diff --git a/ccan/altstack/test/run.c b/ccan/altstack/test/run.c
index adc1020..d0b8d28 100644
--- a/ccan/altstack/test/run.c
+++ b/ccan/altstack/test/run.c
@@ -4,6 +4,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -20,13 +21,13 @@ enum {
};
int fail, call1, call2;
char *m_;
-rlim_t max_;
+rlim_t msz_;
#define e(x) (900+(x))
#define seterr(x) (errno = e(x))
#define setcall(x) ((call1 |= !errno ? (x) : 0), (call2 |= errno || out_ ? (x)
: 0))
#define getrlimit(...) (fail&getrlimit_? (seterr(getrlimit_),
-1) : (setcall(getrlimit_), getrlimit(__VA_ARGS__)))
#define mmap(...) (fail&mmap_ ? (seterr(mmap_),
(void *)-1) : (setcall(mmap_), mmap(__VA_ARGS__)))
-#define munmap(a, b) (fail&munmap_ ? (seterr(munmap_),
-1) : (setcall(munmap_),munmap(m_=(a), max_=(b
+#define munmap(a, b) (fail&munmap_ ? (seterr(munmap_),
-1) : (setcall(munmap_),munmap(m_=(a), msz_=(b
#define setrlimit(...) (fail&setrlimit_? (seterr(setrlimit_),
-1) : (setcall(setrlimit_), setrlimit(__VA_ARGS__)))
#define sigaltstack(...) (fail&sigaltstack_ ?
(seterr(sigaltstack_),-1) : (setcall(sigaltstack_),
sigaltstack(__VA_ARGS__)))
#define sigaction(...) (fail&sigaction_? (seterr(sigaction_),
-1) : (setcall(sigaction_), sigaction(__VA_ARGS__)))
@@ -58,7 +59,9 @@ static void *wrap(void *i)
int main(void)
{
- plan_tests(16);
+ long pgsz = sysconf(_SC_PAGESIZE);
+
+ plan_tests(17);
#define chkfail(x,
[ccan] [PATCH] altstack: New module
altstack - run a function with a dedicated stack, and then release the memory
Signed-off-by: Dan Good
---
Makefile-ccan| 1 +
ccan/altstack/LICENSE| 1 +
ccan/altstack/_info | 125
ccan/altstack/altstack.c | 124
ccan/altstack/altstack.h | 114 +
ccan/altstack/test/run.c | 144 +++
6 files changed, 509 insertions(+)
create mode 12 ccan/altstack/LICENSE
create mode 100644 ccan/altstack/_info
create mode 100644 ccan/altstack/altstack.c
create mode 100644 ccan/altstack/altstack.h
create mode 100644 ccan/altstack/test/run.c
diff --git a/Makefile-ccan b/Makefile-ccan
index 9469334..2c80720 100644
--- a/Makefile-ccan
+++ b/Makefile-ccan
@@ -33,6 +33,7 @@ MODS_NO_SRC := alignof \
# No external dependencies, with C code:
MODS_WITH_SRC := aga \
agar \
+ altstack \
antithread \
antithread/alloc \
asort \
diff --git a/ccan/altstack/LICENSE b/ccan/altstack/LICENSE
new file mode 12
index 000..4f8ee74
--- /dev/null
+++ b/ccan/altstack/LICENSE
@@ -0,0 +1 @@
+../../licenses/APACHE-2
\ No newline at end of file
diff --git a/ccan/altstack/_info b/ccan/altstack/_info
new file mode 100644
index 000..1b03770
--- /dev/null
+++ b/ccan/altstack/_info
@@ -0,0 +1,125 @@
+#include "config.h"
+#include
+#include
+
+/**
+ *
+ * altstack - run a function with a dedicated stack, and then release the
memory
+ *
+ * C99 introduced variable length arrays to make the language easier to use
+ * and more efficient. Many regard VLA's with distrust due to fear of stack
+ * overflow. The same fear causes many to shy away from recursion.
+ *
+ * altstack seeks to liberate us from this fear. altstack creates a dedicated
stack,
+ * limited to a specified maximum size, runs a given function using this
stack, and
+ * afterwards releases the memory back to the system.
+ *
+ * altstack provides a way to obtain current stack usage and a way to fail
gracefully.
+ *
+ * altstack is implemented for x86-64 only.
+ *
+ * Example:
+ * // allocate a VLA on a dedicated stack and show memory usage
+ * #include
+ * #include
+ * #include
+ * #include
+ * #include
+ * #include
+ * #include
+ *
+ * #define ok(x) ({ int __r = (x); if (__r == -1) err(1, #x); __r; })
+ *
+ * char maps[128], rss[128];
+ *
+ * static void stack_used(void) {
+ * fprintf(stderr, "stack used: %ld\n", altstack_used());
+ * }
+ *
+ * static void *fn(void *arg)
+ * {
+ * ok(system(maps));
+ *
+ * stack_used();
+ * ok(system(rss));
+ *
+ * char p[(long) arg];
+ *
+ * stack_used();
+ * ok(system(rss));
+ *
+ * memset(p, 0, sizeof(p));
+ *
+ * stack_used();
+ * ok(system(rss));
+ *
+ * return (void *) 0xaced;
+ * }
+ *
+ * int main(int argc, char *argv[])
+ * {
+ * long stk_max, vla_sz;
+ * int ret;
+ * void *out;
+ *
+ * assert(argc == 3);
+ * stk_max = strtol(argv[1], 0, 0) * 1024 * 1024;
+ * vla_sz = strtol(argv[2], 0, 0) * 1024 * 1024;
+ * assert(stk_max > 0 && vla_sz > 0);
+ *
+ * snprintf(maps, sizeof(maps), "egrep '\\[stack' /proc/%d/maps",
getpid());
+ * snprintf(rss, sizeof(rss), "egrep '^VmRSS' /proc/%d/status",
getpid());
+ *
+ * ok(system(maps));
+ * ok(system(rss));
+ *
+ * ret = altstack(stk_max, fn, (void *) vla_sz, &out);
+ *
+ * ok(system(maps));
+ * ok(system(rss));
+ *
+ * if (ret)
+ * altstack_perror();
+ * fprintf(stderr, "altstack return: %d, fn return: %p\n", ret,
out);
+ *
+ * return 0;
+ * }
+ * // $ ./foo 1024 512
+ * // 7ffeb59a9000-7ffeb59ca000 rw-p 00:00 0
[stack]
+ * // VmRSS: 760 kB
+ * // 7f9cb6005000-7f9cf6004000 rw-p 00:00 0
[stack:25891]
+ * // stack used: 56
+ * // VmRSS: 760 kB
+ * // stack used: 536870968
+ * // VmRSS: 760 kB
+ * // stack used: 536870968
+ * // VmRSS:525500 kB
+ * // 7ffeb59a9000-7ffeb59ca000 rw-p 00:00 0
[stack]
+ * // VmRSS: 1332 kB
+ * // altstack return: 0, fn return: 0xaced
+ * //
+ * // $ ./foo 512 1024
+ * // 7ffd62bd-7ffd62bf1000 rw-p 00:00 0
[stack]
+ * // VmRSS: 700 kB
+ * // 7f0d3bef6000-7f0d5bef5000 rw-p 00:00 0
[stack:25900]
+ * // stack used:
[ccan] [PATCH] rszshm: New module
rszshm - resizable pointer-safe shared memory
Signed-off-by: Dan Good
---
Makefile-ccan | 1 +
ccan/rszshm/LICENSE| 1 +
ccan/rszshm/_info | 96
ccan/rszshm/rszshm.c | 240 +++
ccan/rszshm/rszshm.h | 301 +
ccan/rszshm/test/run.c | 194 +++
6 files changed, 833 insertions(+)
create mode 12 ccan/rszshm/LICENSE
create mode 100644 ccan/rszshm/_info
create mode 100644 ccan/rszshm/rszshm.c
create mode 100644 ccan/rszshm/rszshm.h
create mode 100644 ccan/rszshm/test/run.c
diff --git a/Makefile-ccan b/Makefile-ccan
index 8d99bbf..9469334 100644
--- a/Makefile-ccan
+++ b/Makefile-ccan
@@ -100,6 +100,7 @@ MODS_WITH_SRC := aga \
rbuf \
read_write_all \
rfc822 \
+ rszshm \
siphash \
sparse_bsearch \
str \
diff --git a/ccan/rszshm/LICENSE b/ccan/rszshm/LICENSE
new file mode 12
index 000..4f8ee74
--- /dev/null
+++ b/ccan/rszshm/LICENSE
@@ -0,0 +1 @@
+../../licenses/APACHE-2
\ No newline at end of file
diff --git a/ccan/rszshm/_info b/ccan/rszshm/_info
new file mode 100644
index 000..2134e67
--- /dev/null
+++ b/ccan/rszshm/_info
@@ -0,0 +1,96 @@
+#include "config.h"
+#include
+#include
+
+/**
+ * rszshm - resizable pointer-safe shared memory
+ *
+ * If two separate processes have shared mappings of the same file at the
+ * same address, then pointers to addresses within the region can be shared
+ * between the processes and safely dereferenced.
+ *
+ * Mapping to the same address in unrelated processes is nontrivial. One can
+ * request a specific address, but mmap will return another in case of an
+ * overlap. One can require a specific address, but mmap will unmap anything
+ * overlapped. On Linux boxes it can be seen that the used addresses clump
+ * at either end of the address range. rszshm tries to mmap in the middle
+ * of the address range, and checks if the requested address matches the
+ * returned address. If not, additional addresses are tried. Once mapped,
+ * the address is recorded to a header in the file. Another process reads the
+ * header and requests the saved address from mmap. If the returned address
+ * matches, work proceeds. While the defaults provide a propitious search,
+ * all the search parameters may be specified.
+ *
+ * To accommodate resizing, rszshm first maps a large, private, noreserve map.
+ * This serves to claim a span of addresses. The shared file mapping then
+ * overlays the beginning of the span. Later calls to extend the mapping
+ * overlay more of the span. Attempts to extend beyond the end of the span
+ * return an error.
+ *
+ * Example:
+ * // fork x times, grow and fill shared memory cooperatively
+ * #include
+ * #include
+ * #include
+ * #include
+ * #include
+ * #include
+ *
+ * #define ok(x) ({ int n = (x); if (n == -1) err(1, "%s", #x); n; })
+ *
+ * int main(int argc, char *argv[]) {
+ * int pidcnt, stopval, n, i;
+ * struct rszshm *r;
+ * char *m;
+ *
+ * assert(argc == 3);
+ * pidcnt = atoi(argv[1]);
+ * stopval = atoi(argv[2]);
+ * assert(pidcnt > 0 && stopval > 0);
+ *
+ * if (!rszshm_mkm(r, 4096, NULL))
+ * err(1, "rszshm_mkm");
+ *
+ * printf("%s\n", r->fname);
+ *
+ * for (n = 0; n < pidcnt - 1; n++)
+ * if (ok(fork()) == 0)
+ * break;
+ *
+ * m = (char *) r->dat + sizeof(int);
+ * #define next() (__sync_fetch_and_add((int *) r->dat, 1))
+ *
+ * for(i = next(); i < stopval; i = next()) {
+ * if (i >= r->cap - sizeof(int))
+ * ok(rszshm_grow(r));
+ * assert(m[i] == '\0');
+ * m[i] = 'A' + n;
+ * kill(0, 0); // busy work
+ * }
+ *
+ * rszshm_free(r);
+ * return 0;
+ * }
+ * // $ ./foo 8 $((4*1024*1024-28))
+ * // /dev/shm/rszshm_LAsEvt/0
+ * // $ tail -c +29 /dev/shm/rszshm_LAsEvt/0 | sed 's/./&\n/g' | sort |
uniq -c | tr '\n' '\t'; echo
+ * // 515532 A 527251 B512930 C513062 D544326 E545876 F
512936 G522363 H
+ *
+ * License: APACHE-2
+ * Author: Dan Good
+ *
+ * Ccanlint:
+ * // tests use optional macros containing statement expressions
+ * tests_compile_without_features FAIL
+ */
+int main(int argc, char *argv[])
+{
+ /* Expect exactly one argument */
+ if (argc != 2)
+ return 1;
+
+ if (strcmp(argv[1], "depends") == 0)
+ return 0;
+
+ return 1;
+}
diff --git a/ccan
Re: [ccan] deque: New module
Thank you for the feedback. I have a practical question - what is the workflow for making changes to this module, now that it's been merged? Do I clone, then branch, edit, commit, push, merge, and finally mail a patch to the list? I'm still learning git. I've been on the rcs/cvs/svn bandwagon for a long time. I'd like some clarification on the need for portability. Once I read your comments, I came up with an ad-hoc list of platforms I felt I could reasonably support: major Linux distros, FreeBSD, OS X, and OmniOS. I tried the module on each successfully, though ccanlint needed changes to build on everything but Linux, and the failtest based test went rogue on two of the platforms. Compound literals are a feature of C99 (nice article: http://www.drdobbs.com/the-new-c-compound-literals/184401404). Intermingled declarations and code are also from C99. Half of the platforms above use gcc and half use clang. Both gcc and clang support statement expressions. The _XOPEN_SOURCE define in _info is there for getline(3) on Linux which is used solely in the demo code in _info. Is there a set of platforms CCAN targets for support? Is there some way I should document that the code is C99? Would the lines below be the right way to handle the statement expression dependance? #if !HAVE_STATEMENT_EXPR #error Sorry, deque requires statement expressions #endif Thanks. -Dan On Sun, Dec 27, 2015 at 10:45 PM David Gibson wrote: > On Wed, Dec 23, 2015 at 12:11:44PM +, Dan Good wrote: > > deque - type-preserving resizing circular deque > > Concept looks good. There are some fairly minor nits in the > implementation that I've noted below. However, none are serious > enough to hold up merging, so I've applied this. I did modify > trivially to remove some trailing whitespace. > > It would be nice to address some of the comments in follow up > commits, particularly some of the portability issues. > [...] > -- > 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 > ___ ccan mailing list ccan@lists.ozlabs.org https://lists.ozlabs.org/listinfo/ccan
[ccan] deque: New module
deque - type-preserving resizing circular deque
Signed-off-by: Dan Good
---
Makefile-ccan | 1 +
ccan/deque/LICENSE | 1 +
ccan/deque/_info | 140 +++
ccan/deque/deque.c | 91 ++
ccan/deque/deque.h | 252 +
ccan/deque/test/run1.c | 140 +++
ccan/deque/test/run2.c | 59
ccan/deque/test/run3.c | 37
8 files changed, 721 insertions(+)
create mode 12 ccan/deque/LICENSE
create mode 100644 ccan/deque/_info
create mode 100644 ccan/deque/deque.c
create mode 100644 ccan/deque/deque.h
create mode 100644 ccan/deque/test/run1.c
create mode 100644 ccan/deque/test/run2.c
create mode 100644 ccan/deque/test/run3.c
diff --git a/Makefile-ccan b/Makefile-ccan
index 8eac2d7..8d99bbf 100644
--- a/Makefile-ccan
+++ b/Makefile-ccan
@@ -57,6 +57,7 @@ MODS_WITH_SRC := aga \
crypto/shachain \
daemonize \
daemon_with_notify \
+ deque \
dgraph \
eratosthenes \
err \
diff --git a/ccan/deque/LICENSE b/ccan/deque/LICENSE
new file mode 12
index 000..4f8ee74
--- /dev/null
+++ b/ccan/deque/LICENSE
@@ -0,0 +1 @@
+../../licenses/APACHE-2
\ No newline at end of file
diff --git a/ccan/deque/_info b/ccan/deque/_info
new file mode 100644
index 000..b1a47bb
--- /dev/null
+++ b/ccan/deque/_info
@@ -0,0 +1,140 @@
+#include "config.h"
+#include
+#include
+
+/**
+ * deque - type-preserving resizing circular deque
+ *
+ * This is a deque (double-ended queue, pronounced deck) implementation using
+ * a resizing circular buffer. At steady state, deque operations can proceed
+ * perpetually without mallocing. The initial capacity must be specified and
+ * is a lower bound when shrinking. Buffer capacity is doubled at enqueue
+ * to a full deque. Shrink behavior choices are never shrink, shrink to
+ * minimum when the queue is empty, or shrink by half when the queue is at 20%
+ * of capacity. Operation names are in the Perl/Ruby style.
+ *
+ * Example:
+ * // Evaluates arithmetic expressions using Dijkstra's two-stack
algorithm.
+ * // Original:
http://algs4.cs.princeton.edu/13stacks/EvaluateDeluxe.java.html
+ * #define _XOPEN_SOURCE 700
+ * #include
+ * #include
+ * #include
+ * #include
+ * #include
+ *
+ * static double eval(char op, double a, double b)
+ * {
+ * switch (op) {
+ * case '+': return a + b;
+ * case '-': return a - b;
+ * case '/': return a / b;
+ * case '*': return a * b;
+ * }
+ * errx(1, "bad op: %c", op);
+ * }
+ *
+ * char opchr[] = { '(', ')', '+', '-', '*', '/' };
+ * int opprc[] = { 0 , 0 , 1 , 1 , 2 , 2 };
+ *
+ * static int precedence(char op)
+ * {
+ * int i;
+ * for (i = 0; i < sizeof(opchr); i++)
+ * if (opchr[i] == op)
+ * return opprc[i];
+ * return -1;
+ * }
+ *
+ * #define ok(x) ({ int n = (x); if (n == -1) err(1, "%s", #x); n; })
+ *
+ * int main(int argc, char *argv[])
+ * {
+ * DEQ_WRAP(char) *ops;
+ * DEQ_WRAP(double) *vals;
+ * char *ln = NULL, *p, op;
+ * size_t lnsz = 0;
+ * double a, b;
+ * int n;
+ *
+ * ok(deq_new(ops, 8, DEQ_NO_SHRINK));
+ * ok(deq_new(vals, 8, DEQ_NO_SHRINK));
+ *
+ * while (getline(&ln, &lnsz, stdin) > 0) {
+ *
+ * for (p = ln; *p; p++) {
+ * if (isspace(*p))
+ * continue;
+ *
+ * if (precedence(*p) == -1) {
+ * if (sscanf(p, "%lf%n", &a, &n) != 1)
+ * errx(1, "parse fail: %s", p);
+ * ok(deq_push(vals, a));
+ * p += n - 1;
+ * continue;
+ * }
+ *
+ * while (1) {
+ * if (*p == '(' || deq_last(ops, &op) ==
0 || (precedence(*p) > precedence(op))) {
+ * ok(deq_push(ops, *p));
+ * break;
+ * }
+ *
+ * ok(deq_pop(ops, &op));
+ *
+ * if (op == '(') {
+ * assert(*p == ')');
+ * break;
+ *
