Alt+Backspace off-by-one and segfault (UTF-8) #256
Comments
Fix for the bug. |
The same applies here. This is no bug for the 4.6.2.1 release... |
Patch is applied to both HACK_utf8 branches and will be included in the next release |
|
need check it |
|
now all correct |
|
Reopening: same bug is present in 4.7-pre2. Probably it was fixed in the 4.6 branch, but not in the complete UTF-8 rewrite for 4.7. |
Patch attached, please test it thoroughly. (It's slightly different than the one used for 4.6. As a matter of fact, I don't fully understand how that one worked correctly :))
The trick is: p points to the location of the cursor, that is, the first byte of the character it stands under. When moving forward, you have to examine this character and see if it's alphanumerical or not - easy story. When moving backwards, however, you need to check the preceding character, not this one. |
Fix for off-by-one error in mc 4.7-pre2. |
|
Yep, exactly the same bug. |
|
|
branch: 256_widget_backward_kill_word_fix (parent: master) |
See my last [016d79cdceb60faddfb6d798138753efe6d2f96d] |
Ops, I forgot to remove some code: [a12abcf77a94f1bd4bf71b2ef3990d5fa6d4be06] |
|
|
Fixed: [9bec191] |
|
Reopening. Unfortunately still not fixed, and the new behavior might even cause segfault, hence is much more harmful than the old one.
If you type English letters only (e.g. "patch.gz") then Alt+Backspace works as expected, it removes "gz" first, then removes "patch.", then does nothing.
The bug happens when you start using accented letters. Type "áá.éé" for example, and then press Alt+Backspace. It removes ".éé" - already buggy. Press it once again, seems to do nothing. Press once or twice again, still seems to do nothing.
Then start typing normal letters. The first few ones don't appear, but then suddenly they start appearing. If you keep on doing various things in mc, you'll pretty soon get a segmentation fault.
Definitely looks like a buffer underrun.
I can't tell at this moment if my originally propsed patch was already buggy or not. I will take a closer look and try to come up with a fix later, I don't have time right now. |
My previously proposed w/ 4.7-pre2 is already broken. Sorry for that. Working on the fix... |
fix for segfault and other weird errors |
Fix attached.
The for loop, whose purpose is to remove exactly 1 character (hence I don't get why it's a loop, but nevermind) was not UTF8-ready. So if the character preceding the cursor was an accented one, it jumped to the middle of the UTF-8 sequence, causing the rest of the stuff go unpredictable.
Although it *should* never happen (which, as we all know, does not equal to "never happens"), in this case "p" simply jumped over "in->buffer". The function has a "p != in->buffer" check three times, it might it more robust if you replaced that with "p >= in->buffer". This should prevent the segfault, and just stay with a slightly buggy but otherwise harmless alt-backspace behavior, should there be any UTF-8 or similar bugs left. This change is not included in my patch. |
|
Use assert() to catch this? |
|
branch: 256_backward_word_fix |
|
Fixed: [7e30a30] |
|
Important
This issue was migrated from Trac:
egmont(@egmontkob)dborca@….com,egmont@….com(@egmontkob)The UTF-8 patch introduces this regression: Ctrl+Left and Alt+Backspace proceed one more character than they should.
(Consistently with unpatched mc, bash, and probably lots of other tools, Alt+Backspace should remove a sequence of non-alphanumeric characters followed by a sequence of alphanumeric ones, but instead it removes one more non-alphanumeric character at the end. Similar for Ctrl+Left.)
Trivial patch is attached. I've created this patch years ago and posted to mc-devel; probably it's already included in some major Linux distributions as well.
Please apply this fix in your UTF-8 patch. Thanks!
Note
Original attachments:
egmont(@egmontkob) onFeb 5, 2009 at 22:24 UTCegmont(@egmontkob) onSep 19, 2009 at 22:43 UTCegmont(@egmontkob) onNov 4, 2009 at 19:52 UTCThe text was updated successfully, but these errors were encountered: