nmh-workers
[Top] [All Lists]

Re: fmttest(1) Suffers Segmentation Violation.

2021-05-04 09:52:01
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


<Prev in Thread] Current Thread [Next in Thread>