Yes, I think this is fine. Applied, thanks.

On Wed, Dec 21, 2011 at 03:59:39PM -0600, Chris Johnsen wrote:
> A pane can potentially have up to INT_MAX lines of history (limited
> via "history-limit"), but "capture-pane" only accepts values down to
> SHRT_MIN for its "-E" and "-S" options.
> 
> To allow it to capture a pane's full history, negative numbers down
> to at least -INT_MAX should be accepted for these options. Technical
> users might try to use INT_MIN, so we should accept that too.
> Ideally, we should use MIN(INT_MIN, -INT_MAX), but INT_MIN is less
> than -INT_MAX on most platforms, so it should suffice.
> 
> The existing code already handles "below zero" unsigned overflow
> situations.
> 
> The upper bound on the specified line number remains unchanged at
> SHRT_MAX. Technically, a pane's height (gd->sy) could possibly be up
> to USHRT_MAX (from TIOCGWINSZ, winsize.ws_row is usually "unsigned
> short" or "new-session -dy N", where N is limited to USHRT_MAX), but
> allowing line specifications that high would open up the possibility
> of integer overflow on a platform where USHRT_MAX equals UINT_MAX.
> Realistically, it seems highly unlikely that any terminal emulators
> allow heights even close to the smallest conforming SHRT_MAX (much
> less USHRT_MAX, whether it equals UINT_MAX or not), so SHRT_MAX is
> probably an acceptable upper limit.
> 
> ---
> 
> I found this issue while attempting to capture all of a pane's
> history and its currently displayed lines (generically, without
> knowledge of the pane's effective hlimit), by trying to use -1000000
> as a sufficiently large, negative value with "-S". The result was
> that that it only captured the displayed lines (the default, since
> strtonum reported ERANGE). I had to back down to -32768 to make it
> work.
> 
> I am not sure how many users ever actually put "history-limit"
> anywhere between SHRT_MAX and INT_MAX, but it seemed like a minor
> bug not to support such configurations.
> ---
>  trunk/cmd-capture-pane.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/trunk/cmd-capture-pane.c b/trunk/cmd-capture-pane.c
> index 9f2d792..52fc792 100644
> --- a/trunk/cmd-capture-pane.c
> +++ b/trunk/cmd-capture-pane.c
> @@ -59,7 +59,7 @@ cmd_capture_pane_exec(struct cmd *self, struct cmd_ctx *ctx)
>       buf = NULL;
>       len = 0;
>  
> -     n = args_strtonum(args, 'S', SHRT_MIN, SHRT_MAX, &cause);
> +     n = args_strtonum(args, 'S', INT_MIN, SHRT_MAX, &cause);
>       if (cause != NULL) {
>               top = gd->hsize;
>               xfree(cause);
> @@ -70,7 +70,7 @@ cmd_capture_pane_exec(struct cmd *self, struct cmd_ctx *ctx)
>       if (top > gd->hsize + gd->sy - 1)
>               top = gd->hsize + gd->sy - 1;
>  
> -     n = args_strtonum(args, 'E', SHRT_MIN, SHRT_MAX, &cause);
> +     n = args_strtonum(args, 'E', INT_MIN, SHRT_MAX, &cause);
>       if (cause != NULL) {
>               bottom = gd->hsize + gd->sy - 1;
>               xfree(cause);
> -- 
> 1.7.7.4
> 
> 
> ------------------------------------------------------------------------------
> Write once. Port to many.
> Get the SDK and tools to simplify cross-platform app development. Create 
> new or port existing apps to sell to consumers worldwide. Explore the 
> Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
> http://p.sf.net/sfu/intel-appdev
> _______________________________________________
> tmux-users mailing list
> tmux-users@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/tmux-users

------------------------------------------------------------------------------
Write once. Port to many.
Get the SDK and tools to simplify cross-platform app development. Create 
new or port existing apps to sell to consumers worldwide. Explore the 
Intel AppUpSM program developer opportunity. appdeveloper.intel.com/join
http://p.sf.net/sfu/intel-appdev
_______________________________________________
tmux-users mailing list
tmux-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tmux-users

Reply via email to