[mcview] Library/executable symbols not fully viewable

[lastique]

When I press F3 on a library or executable with lots of exported symbols with long names (typical for C++ libraries), mcview does not always allow to scroll down though all symbols and stops halfway.

Steps to reproduce:

1. Find an executable or library with lots of exported symbols with long names. For example, libboost_thread.so.1.62.0.

2. Open it for viewing by pressing F3.

3. Scroll down through symbols by pressing Arrow Down or Page Down key.

4. At some point mcview stops scrolling as if the list of symbols ended. Typically this happens when the last symbol is very long (maybe longer than the console width; I don't know the exact criteria).

5. At this point, if you scroll right by pressing Arrow Right or Ctrl+Arrow Right so that the viewer shows _past_ the end of the last symbol name, mcview allows to continue scrolling down by pressing Arrow Down or Page Down - until the next such point where it stops.

This bug is really annoying because one can't tell if the file has ended or not.

[andrew_b]

Replying to lastique:

4. At some point mcview stops scrolling as if the list of symbols ended. Typically this happens when the last symbol is very long (maybe longer than the console width; I don't know the exact criteria).

The exact criteria is 8K bytes. You can see that in the top-right corner as 8192/8192+.

[andrew_b]

This behavior is not occured in text wrap mode.

I think the problem is in mcview_display_line (src/viewer/ascii.c:593) which is called in mcview_ascii_move_down (src/viewer/ascii.c:936). Probably, we should call mcview_may_still_grow() in ascii.c:593.

Egmont, do you have any idea?

[egmont]

Will take a look soon, hopefully tonight. Thanks for looping me in! I could reproduce the problem. Do we know if it was my big rewrite that broke it? I'll check it.

I sometimes had a feeling that search was not reading the entire output, but, my bad, never cared enough to track down and file a bugreport. Could be caused by the same issue.

[egmont]

It was indeed broken by me with the big viewer rewrite (#3250, bcb09f6).

[egmont]

Trying to understand what's going on, to come up with the right fix:

mcview_display_line() contains two branches commented as "Optimization: Fast forward to the end of the line, rather than carefully parsing and then not actually displaying it."

This method does not only display the requested row of the screen, but also updates the state from where the next visible row can be painted. The state contains file offset, plus other attributes (e.g. nroff color) that we might need to carry.

In wrap mode, we need to stop right after displaying the visible row, there we have the state from where displaying the next visible row will continue. There's nothing else to do.

In nowrap mode, we need to skip to the end of the file's line. This is what the optimization branches do. This optimization could be cut off, in that case the "normal" body of the method would walk through the characters one by one, adjusting the state machine (e.g. tracking nroff changes etc.), just to drop this whole thing at the end (since no formatting state is carried across newlines).

A workaround for the current problem is to disable the ~10 lines of code at the top of this method commented as "Optimization....".

The core of the problem is that the "normal" path calls mcview_next_combining_char_sequence() which eventually uses mcview_get_byte() or mcview_get_utf() which take care of extending the growbuf if necessary. The shortcut optimized path calls mcview_eol() instead which does not. In fact, eventually it also calls mcview_get_byte(), however, beforehand it explicitly bails out if the end of the current buffer is reached, without trying to grow it.

Hence I'm wondering if the correct solution would be to fix mcview_eol() to grow if necessary.

This might fix some other weird corner cases as well. E.g. when the actual viewer area is only a single row high and you press Ctrl+E to get to the end of that line. (The bug here would be more prominent if Ctrl+E right-aligned the last visible row rather than the first. Then I guess we could spot a bug with sane window sizes as well.)

andrew_b, what do you think, does this approach sound reasonable to you?

[andrew_b]

Replying to egmont:

andrew_b, what do you think, does this approach sound reasonable to you?

Yes, it is reasonable for me, but I'm not so deeply in the viewer code.

[egmont]

(Forget the paragraph about other bugs/corner cases, it was stupid.)

[egmont]

Fix

[egmont]

Could you guys please test this patch?

I haven't paid too much attention to possible side effects and haven't tested it thoroughly.

Also note that this patch does _not_ fix the search issue I've referred to, that seems to be a different bug. I've reported that as #3819.

[lastique]

I patched the 4.8.18 that is shipped with my Ubuntu 17.04 and a quick test shows the problem is fixed. I'll keep using the patched version and see if there are any problems.

Thanks guys.

[andrew_b]

Branch: 3817_mcview_grow_buffer
changeset:8d25e94bd59cde4b534ce6136e2eeb37c26dbe7d

(I have some problem with correct commit comment.)

[andrew_b]

Merged to master: [71c8f5bedceafbf1e1253ad8884e7dc01856bebc].