[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Nmh-workers] sbr/fmt_scan.c: cpstripped(): assert(w >= 0) Failure.

From: Ralph Corderoy
Subject: Re: [Nmh-workers] sbr/fmt_scan.c: cpstripped(): assert(w >= 0) Failure.
Date: Mon, 01 May 2017 14:40:25 +0100

Hi David,

> > > I noticed that the altstr = NULL; in cptrimmed(), line 197, is
> > > inside the w >=0 block.  I think it should be outside.
> >
> > The else block for that condition effectively returns so the
> > un-NULL'd altstr isn't used again.  (They should be swapped.  :-)
> That's in cpstripped(), right?  There isn't an else block in
> cptrimmed().

Right.  ctag-ing around, like I do, I didn't realise there were two
adjacent, similar named, similar bodied, functions and that we'd
switched topics.  :-)

> I don't have time to look at these for a while.

Yep, time's a problem.

> But I agree that they need to be cleaned up.

Yes.  I was trying to trigger that altstr not being reset to NULL that
you pointed out and found another rabbit-hole instead.  It seems
right-justification affects the output buffer so far, not the thing just
added to it.

    uip/fmttest -raw \
        -format '.%-7(putstrf{text}).%7(putstrf{text}).\n' foo

    .    foo.foo    .    1.6-branchpoint-133-g11e2fb2d
        .foo.foo    .    1.6-branchpoint-134-g92128dac
        .foo.foo    .    1.6-branchpoint-1314-gab6e5468

test/format/test-rightjustify doesn't tickle this case.  I was thinking
it might be nice if test/common.sh supported `expected failure' or `bug'
test cases.  It would run the test, still expect the given, faulty,
output, but indicate it was an expected failure.  Fixing the bug would
break the test case.

> > Have added a to-do item to spell-check comments, with that as a
> > sample to test my shell skills.
> misspell_fixer, https://github.com/vlajos/misspell_fixer, was run on
> the code last year and found a bunch, which were fixed.  I just ran
> it, no complaints.

I had a go and fixed some more, 9514ca81bd1, but didn't spot `alstr'.
:-)  But then it might die in the merge/fix of cpstripped() and

Cheers, Ralph.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]