Re: fmttest(1) Suffers Segmentation Violation.

[Ken Hornstein]

>> Oh, strangely enough ... it's because it's trying to call
>> seq_setprev() with no messages set.
>
>Not for me as I don't have Previous-Sequence set.  git-grep(1) suggests
>our test suite doesn't set it either?

Well, geez, Ralph, you COULD run a debugger and figure out what's going
wrong yourself! :-)

What is actually happening, for me at least, is that fmttest is calling
seq_setprev(), which calls seq_addsel(), which ends up crashing in
is_selected().  is_selected() is a macro which expands to:

        bvector_at(msgstat(mp, msgnum), SELECTED)

msgstat is also a macro which expands to:

        ((mp)->msgstats + (n) - mp->lowoff)

But in our case, msgnum is 0.  This causes a segfault because lowoff is 1,
and this math means the pointer to msgstats is before the start of the
msgstats array.  Why is msgnum 0?  Because of this loop:

    for (msgnum = mp->lowsel; msgnum <= mp->hghsel; msgnum++)
        if (is_selected (mp, msgnum))
            add_sequence (mp, i, msgnum);

The problem is in THIS case, mp->lowsel is zero (mp->hghsel is also zero).
Because of the <=, it means this loop is executed at least once.

So, obviously, you're being tripped up by the same loop that appears
further down in fmttest.c.

Now, this makes me wonder what the REAL bug is.  This loop appears
everywhere in nmh.  If there are no selected messages in a folder, how
should this be represented in nmh?  Right now when folder_read() is
called and the folder structure is initialized, mp->lowsel is 0.
But mp->hghsel is also 0.  Should we initialize mp->hghsel to -1, so
the loop never goes around?  Should we make it so is_selected()
handles the case where it is called with an out-of-range message
number?  (I mean, probably).  Seems like an underspecified API problem.

>> And that suggests to me if you don't give a message on the command
>> line, it should default to 'cur' like other nmh programs, unless
>> people think it makes sense to fail in that case.
>
>fmttest doesn't alter the message so I think ‘cur’ is the correct
>default value of ‘msgs’.

Right, but I think the bug that got us here should also be fixed.

>> (And I realize from reading the man page I never documented what
>> -file/-nofile means).
>
>fmttest(1) here says
>
>   ‘If the -file switch is given, the arguments are interpreted as
>    filenames instead of message numbers, but otherwise the behavior is
>    the same (except that the %(msg), %(cur), and %(unseen) function
>    escapes will not provide any useful information).’

Ah, whew!  I did document that after all!

--Ken