Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
On 2015-01-25 00:18 +, Nicholas Marriott wrote: Unfortunately I don't have the backtrace anymore, I was busy and the easiest thing at the time was to revert it, it was definitely under grid_view_insert_cells and I couldn't see anything obvious. I've kept running my main instance with this patch and managed to hit the problem. After some trial and error I've managed to reproduce and track down the problem. I made a wrong assumption, namely this one: (px) is always strictly smaller than (cellsize) This is not true. Here's a simple repro where this patch segfaults tmux: echo -e '\e[C\e[@' # cursor right, insert 1 character So basically my patch needs an extra line like much like the delete has: if (sx px+nx) sx = px+nx; Now the (sx - px - nx) expression will never underflow. Having said that, I don't mind that this is not in tmux, just wanted to point out the bug for historical reasons. :) -- Balazs -- Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=190641631iu=/4140/ostg.clktrk ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
Hi Unfortunately I don't have the backtrace anymore, I was busy and the easiest thing at the time was to revert it, it was definitely under grid_view_insert_cells and I couldn't see anything obvious. I only saw it very occasionally and never managed to reproduce - I know it was crashing regularly to someone else in zenicb in emacs on OpenBSD along with occasional screen corruption - I suspect it is some edge case that emacs sends under rare circumstances. tools/fuzz.c (possibly modified to favour insert/delete cells?) might turn something up but if you don't mind about the feature, don't worry about it - we can just leave it reverted. We'd need to be very sure it was right before it went in again anyway. On Sat, Jan 24, 2015 at 11:53:20PM +, Balazs Kezes wrote: On 2015-01-06 21:31 +, Nicholas Marriott wrote: This was causing random crashes, so it's clearly still wrong and I've reverted back to before any of these changes. I don't have time to look into it further but maybe you want to have a look (with valgrind maybe?). I had no luck reproducing. Neither did valgrind show any suspicious behavior. I presume you don't have a stack trace, right? Anyways, I'm fine having this feature reverted as it is harmless and wasn't that important for me as my other patch from this series. :) Thanks! -- Balazs -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
On 2015-01-06 21:31 +, Nicholas Marriott wrote: This was causing random crashes, so it's clearly still wrong and I've reverted back to before any of these changes. I don't have time to look into it further but maybe you want to have a look (with valgrind maybe?). I had no luck reproducing. Neither did valgrind show any suspicious behavior. I presume you don't have a stack trace, right? Anyways, I'm fine having this feature reverted as it is harmless and wasn't that important for me as my other patch from this series. :) Thanks! -- Balazs -- New Year. New Location. New Benefits. New Data Center in Ashburn, VA. GigeNET is offering a free month of service with a new server in Ashburn. Choose from 2 high performing configs, both with 100TB of bandwidth. Higher redundancy.Lower latency.Increased capacity.Completely compliant. http://p.sf.net/sfu/gigenet ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
This was causing random crashes, so it's clearly still wrong and I've reverted back to before any of these changes. I don't have time to look into it further but maybe you want to have a look (with valgrind maybe?). Cheers On Mon, Dec 01, 2014 at 10:23:54PM +, Nicholas Marriott wrote: I think this is alright, the calculations seem sensible anyway. On Sat, Nov 29, 2014 at 09:31:10PM +, Balazs Kezes wrote: Disregard my previous patch, after a little bit of thinking I came up with this: diff --git a/grid-view.c b/grid-view.c index a34c5a0..0b890d8 100644 --- a/grid-view.c +++ b/grid-view.c @@ -184,9 +184,10 @@ grid_view_insert_cells(struct grid *gd, u_int px, u_int py, u_int nx) px = grid_view_x(gd, px); py = grid_view_y(gd, py); - sx = grid_view_x(gd, gd-linedata[py].cellsize); - if (sx px + nx) - sx = px + nx; + if (gd-linedata[py].cellsize + nx gd-sx) + sx = grid_view_x(gd, gd-linedata[py].cellsize + nx); + else + sx = grid_view_x(gd, gd-sx); if (px == sx - 1) grid_clear(gd, px, py, 1, 1); For insertion we need to size it to (cellsize+nx) because we have (nx) new characters and now the expression (sx-px-nx) cannot go below zero because (sx-nx) is just (cellsize) and (px) is always strictly smaller than (cellsize). For the other branch, (gd-sx-px-nx = 0) is guaranteed by the calling site. And I believe the deletion part is already handled well. Let me know if there's a better way of testing this than trial and error. -- Balazs -- Dive into the World of Parallel Programming! The Go Parallel Website, sponsored by Intel and developed in partnership with Slashdot Media, is your hub for all things parallel software development, from weekly thought leadership blogs to news, videos, case studies, tutorials and more. Take a look and join the conversation now. http://goparallel.sourceforge.net ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
I think this is alright, the calculations seem sensible anyway. On Sat, Nov 29, 2014 at 09:31:10PM +, Balazs Kezes wrote: Disregard my previous patch, after a little bit of thinking I came up with this: diff --git a/grid-view.c b/grid-view.c index a34c5a0..0b890d8 100644 --- a/grid-view.c +++ b/grid-view.c @@ -184,9 +184,10 @@ grid_view_insert_cells(struct grid *gd, u_int px, u_int py, u_int nx) px = grid_view_x(gd, px); py = grid_view_y(gd, py); - sx = grid_view_x(gd, gd-linedata[py].cellsize); - if (sx px + nx) - sx = px + nx; + if (gd-linedata[py].cellsize + nx gd-sx) + sx = grid_view_x(gd, gd-linedata[py].cellsize + nx); + else + sx = grid_view_x(gd, gd-sx); if (px == sx - 1) grid_clear(gd, px, py, 1, 1); For insertion we need to size it to (cellsize+nx) because we have (nx) new characters and now the expression (sx-px-nx) cannot go below zero because (sx-nx) is just (cellsize) and (px) is always strictly smaller than (cellsize). For the other branch, (gd-sx-px-nx = 0) is guaranteed by the calling site. And I believe the deletion part is already handled well. Let me know if there's a better way of testing this than trial and error. -- Balazs -- Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=157005751iu=/4140/ostg.clktrk ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
Disregard my previous patch, after a little bit of thinking I came up with this: diff --git a/grid-view.c b/grid-view.c index a34c5a0..0b890d8 100644 --- a/grid-view.c +++ b/grid-view.c @@ -184,9 +184,10 @@ grid_view_insert_cells(struct grid *gd, u_int px, u_int py, u_int nx) px = grid_view_x(gd, px); py = grid_view_y(gd, py); - sx = grid_view_x(gd, gd-linedata[py].cellsize); - if (sx px + nx) - sx = px + nx; + if (gd-linedata[py].cellsize + nx gd-sx) + sx = grid_view_x(gd, gd-linedata[py].cellsize + nx); + else + sx = grid_view_x(gd, gd-sx); if (px == sx - 1) grid_clear(gd, px, py, 1, 1); For insertion we need to size it to (cellsize+nx) because we have (nx) new characters and now the expression (sx-px-nx) cannot go below zero because (sx-nx) is just (cellsize) and (px) is always strictly smaller than (cellsize). For the other branch, (gd-sx-px-nx = 0) is guaranteed by the calling site. And I believe the deletion part is already handled well. Let me know if there's a better way of testing this than trial and error. -- Balazs -- Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=157005751iu=/4140/ostg.clktrk ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
Hey Nick! Sorry, but this is still broken. :( You can reproduce it in bash, if you type a b then go to the beginning press c: you can see that the b has disappeared and all you can see is ca. Here's a patch to fix this. diff --git a/grid-view.c b/grid-view.c index a34c5a0..defb86d 100644 --- a/grid-view.c +++ b/grid-view.c @@ -184,7 +184,10 @@ grid_view_insert_cells(struct grid *gd, u_int px, u_int py, u_int nx) px = grid_view_x(gd, px); py = grid_view_y(gd, py); - sx = grid_view_x(gd, gd-linedata[py].cellsize); + sx = gd-linedata[py].cellsize + nx; + if (gd-sx sx) + sx = gd-sx; + sx = grid_view_x(gd, sx); if (sx px + nx) sx = px + nx; @@ -203,7 +206,10 @@ grid_view_delete_cells(struct grid *gd, u_int px, u_int py, u_int nx) px = grid_view_x(gd, px); py = grid_view_y(gd, py); - sx = grid_view_x(gd, gd-linedata[py].cellsize); + sx = gd-linedata[py].cellsize + nx; + if (gd-sx sx) + sx = gd-sx; + sx = grid_view_x(gd, sx); if (sx px + nx) sx = px + nx; But I'm just complicating stuff and I'm not even sure this will cover all cases based on my previous track record so it's fine if you just revert my patches for these two functions. I'd need more time to rethink my approach to fix the original bug in a nice way but I don't think I'll be able to do that in the very near future. Thanks! -- Balazs -- Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server from Actuate! Instantly Supercharge Your Business Reports and Dashboards with Interactivity, Sharing, Native Excel Exports, App Integration more Get technology previously reserved for billion-dollar corporations, FREE http://pubads.g.doubleclick.net/gampad/clk?id=157005751iu=/4140/ostg.clktrk ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
Looks good, applied thanks On Mon, Nov 10, 2014 at 08:55:38PM +, Balazs Kezes wrote: On 2014-11-10 20:04 +, Nicholas Marriott wrote: I reverted the grid-view.c change because it breaks insertion. Try: $ tmux new 'tput ich 10' Oh, sorry about that. What a silly mistake, the 4th parameter underflows in grid_move_cells: ... #1 0x0042a3f9 in log_fatal (msg=0x46e461 %s: %s) at log.c:105 #2 0x00458dba in xreallocarray (oldptr=0x0, nmemb=4294967286, size=14) at xmalloc.c:92 #3 0x0042073b in grid_expand_line (gd=0x1af9790, py=1, sx=4294967286) at grid.c:226 #4 0x00420d89 in grid_move_cells (gd=0x1af9790, dx=10, px=0, py=1, nx=4294967286) at grid.c:381 #5 0x00420182 in grid_view_insert_cells (gd=0x1af9790, px=0, py=1, nx=10) at grid-view.c:192 #6 0x00432d14 in screen_write_insertcharacter (ctx=0x1af9528, nx=10) at screen-write.c:543 ... How about this as the bugfix: diff --git a/grid-view.c b/grid-view.c index 45737e3..39017c1 100644 --- a/grid-view.c +++ b/grid-view.c @@ -185,6 +185,8 @@ grid_view_insert_cells(struct grid *gd, u_int px, u_int py, u_int nx) py = grid_view_y(gd, py); sx = grid_view_x(gd, gd-linedata[py].cellsize); + if (sx px + nx) + sx = px + nx; if (px == sx - 1) grid_clear(gd, px, py, 1, 1); @@ -202,6 +204,8 @@ grid_view_delete_cells(struct grid *gd, u_int px, u_int py, u_int nx) py = grid_view_y(gd, py); sx = grid_view_x(gd, gd-linedata[py].cellsize); + if (sx px + nx) + sx = px + nx; grid_move_cells(gd, px, px + nx, py, sx - px - nx); grid_clear(gd, sx - nx, py, px + nx - (sx - nx), 1); -- Balazs -- Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://pubads.g.doubleclick.net/gampad/clk?id=154624111iu=/4140/ostg.clktrk ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
Hi I reverted the grid-view.c change because it breaks insertion. Try: $ tmux new 'tput ich 10' I guess probably the line does need to be extended a bit... On Sat, Nov 08, 2014 at 12:58:52PM +, Nicholas Marriott wrote: Both this and the other one make sense to me - applied, thanks! On Fri, Nov 07, 2014 at 04:31:10PM +, Balazs Kezes wrote: Steps to reproduce: 1. Create a vertical split. 2. Assuming we are running bash in the left pane, enter this: echo -e 'xy\e[D\e[@' # or echo -e 'xyz\e[2D\e[P' 3. Make the left pane smaller: tmux resize-pane -L 5 4. Observe the extra newline. This is ugly. This happens because whenever insert/delete characters in a line, we extend that line to full width with blanks at the end. This patch will use the real length instead. This is especially annoying if you use readline's insert-comment a lot which uses this facility to insert the comment at the beginning of the line. --- grid-view.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/grid-view.c b/grid-view.c index e75b604..45737e3 100644 --- a/grid-view.c +++ b/grid-view.c @@ -184,7 +184,7 @@ grid_view_insert_cells(struct grid *gd, u_int px, u_int py, u_int nx) px = grid_view_x(gd, px); py = grid_view_y(gd, py); - sx = grid_view_x(gd, gd-sx); + sx = grid_view_x(gd, gd-linedata[py].cellsize); if (px == sx - 1) grid_clear(gd, px, py, 1, 1); @@ -201,7 +201,7 @@ grid_view_delete_cells(struct grid *gd, u_int px, u_int py, u_int nx) px = grid_view_x(gd, px); py = grid_view_y(gd, py); - sx = grid_view_x(gd, gd-sx); + sx = grid_view_x(gd, gd-linedata[py].cellsize); grid_move_cells(gd, px, px + nx, py, sx - px - nx); grid_clear(gd, sx - nx, py, px + nx - (sx - nx), 1); -- 2.1.3 -- ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users -- Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://pubads.g.doubleclick.net/gampad/clk?id=154624111iu=/4140/ostg.clktrk ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
Re: [PATCH 1/2] Fix broken reflowing after insert/delete characters
On 2014-11-10 20:04 +, Nicholas Marriott wrote: I reverted the grid-view.c change because it breaks insertion. Try: $ tmux new 'tput ich 10' Oh, sorry about that. What a silly mistake, the 4th parameter underflows in grid_move_cells: ... #1 0x0042a3f9 in log_fatal (msg=0x46e461 %s: %s) at log.c:105 #2 0x00458dba in xreallocarray (oldptr=0x0, nmemb=4294967286, size=14) at xmalloc.c:92 #3 0x0042073b in grid_expand_line (gd=0x1af9790, py=1, sx=4294967286) at grid.c:226 #4 0x00420d89 in grid_move_cells (gd=0x1af9790, dx=10, px=0, py=1, nx=4294967286) at grid.c:381 #5 0x00420182 in grid_view_insert_cells (gd=0x1af9790, px=0, py=1, nx=10) at grid-view.c:192 #6 0x00432d14 in screen_write_insertcharacter (ctx=0x1af9528, nx=10) at screen-write.c:543 ... How about this as the bugfix: diff --git a/grid-view.c b/grid-view.c index 45737e3..39017c1 100644 --- a/grid-view.c +++ b/grid-view.c @@ -185,6 +185,8 @@ grid_view_insert_cells(struct grid *gd, u_int px, u_int py, u_int nx) py = grid_view_y(gd, py); sx = grid_view_x(gd, gd-linedata[py].cellsize); + if (sx px + nx) + sx = px + nx; if (px == sx - 1) grid_clear(gd, px, py, 1, 1); @@ -202,6 +204,8 @@ grid_view_delete_cells(struct grid *gd, u_int px, u_int py, u_int nx) py = grid_view_y(gd, py); sx = grid_view_x(gd, gd-linedata[py].cellsize); + if (sx px + nx) + sx = px + nx; grid_move_cells(gd, px, px + nx, py, sx - px - nx); grid_clear(gd, sx - nx, py, px + nx - (sx - nx), 1); -- Balazs -- Comprehensive Server Monitoring with Site24x7. Monitor 10 servers for $9/Month. Get alerted through email, SMS, voice calls or mobile push notifications. Take corrective actions from your mobile device. http://pubads.g.doubleclick.net/gampad/clk?id=154624111iu=/4140/ostg.clktrk ___ tmux-users mailing list tmux-users@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/tmux-users
