Patch 8.2.4850

[Bram Moolenaar]

Patch 8.2.4850
Problem: Mksession mixes up "tabpages" and "curdir" arguments.
Solution: Correct logic for storing tabpage in session. (closes #10312)
Files: src/session.c, src/testdir/test_mksession.vim


*** ../vim-8.2.4849/src/session.c 2022-03-29 11:56:57.557660671 +0100
--- src/session.c 2022-04-30 15:39:17.135279912 +0100
***************
*** 623,635 ****
win_T *wp;
char_u *sname;
win_T *edited_win = NULL;
- int tabnr;
int restore_stal = FALSE;
win_T *tab_firstwin;
frame_T *tab_topframe;
int cur_arg_idx = 0;
int next_arg_idx = 0;
int ret = FAIL;
#ifdef FEAT_TERMINAL
hashtab_T terminal_bufs;

--- 623,635 ----
win_T *wp;
char_u *sname;
win_T *edited_win = NULL;
int restore_stal = FALSE;
win_T *tab_firstwin;
frame_T *tab_topframe;
int cur_arg_idx = 0;
int next_arg_idx = 0;
int ret = FAIL;
+ tabpage_T *tp;
#ifdef FEAT_TERMINAL
hashtab_T terminal_bufs;

***************
*** 755,772 ****
restore_stal = TRUE;
}

- // May repeat putting Windows for each tab, when "tabpages" is in
- // 'sessionoptions'.
- // Don't use goto_tabpage(), it may change directory and trigger
- // autocommands.
- tab_firstwin = firstwin; // first window in tab page "tabnr"
- tab_topframe = topframe;
if ((ssop_flags & SSOP_TABPAGES))
{
! tabpage_T *tp;
!
! // Similar to ses_win_rec() below, populate the tab pages first so
! // later local options won't be copied to the new tabs.
FOR_ALL_TABPAGES(tp)
// Use `bufhidden=wipe` to remove empty "placeholder" buffers once
// they are not needed. This prevents creating extra buffers (see
--- 755,765 ----
restore_stal = TRUE;
}

if ((ssop_flags & SSOP_TABPAGES))
{
! // "tabpages" is in 'sessionoptions': Similar to ses_win_rec() below,
! // populate the tab pages first so later local options won't be copied
! // to the new tabs.
FOR_ALL_TABPAGES(tp)
// Use `bufhidden=wipe` to remove empty "placeholder" buffers once
// they are not needed. This prevents creating extra buffers (see
***************
*** 777,794 ****
if (first_tabpage->tp_next != NULL && put_line(fd, "tabrewind") == FAIL)
goto fail;
}
! for (tabnr = 1; ; ++tabnr)
{
- tabpage_T *tp = NULL;
int need_tabnext = FALSE;
int cnr = 1;

if ((ssop_flags & SSOP_TABPAGES))
{
- tp = find_tabpage(tabnr);
-
- if (tp == NULL)
- break; // done all tab pages
if (tp == curtab)
{
tab_firstwin = firstwin;
--- 770,789 ----
if (first_tabpage->tp_next != NULL && put_line(fd, "tabrewind") == FAIL)
goto fail;
}
!
! // Assume "tabpages" is in 'sessionoptions'. If not then we only do
! // "curtab" and bail out of the loop.
! FOR_ALL_TABPAGES(tp)
{
int need_tabnext = FALSE;
int cnr = 1;

+ // May repeat putting Windows for each tab, when "tabpages" is in
+ // 'sessionoptions'.
+ // Don't use goto_tabpage(), it may change directory and trigger
+ // autocommands.
if ((ssop_flags & SSOP_TABPAGES))
{
if (tp == curtab)
{
tab_firstwin = firstwin;
***************
*** 799,807 ****
tab_firstwin = tp->tp_firstwin;
tab_topframe = tp->tp_topframe;
}
! if (tabnr > 1)
need_tabnext = TRUE;
}

// Before creating the window layout, try loading one file. If this
// is aborted we don't end up with a number of useless windows.
--- 794,808 ----
tab_firstwin = tp->tp_firstwin;
tab_topframe = tp->tp_topframe;
}
! if (tp != first_tabpage)
need_tabnext = TRUE;
}
+ else
+ {
+ tp = curtab;
+ tab_firstwin = firstwin;
+ tab_topframe = topframe;
+ }

// Before creating the window layout, try loading one file. If this
// is aborted we don't end up with a number of useless windows.
***************
*** 893,903 ****
// Restore the tab-local working directory if specified
// Do this before the windows, so that the window-local directory can
// override the tab-local directory.
! if (tp != NULL && tp->tp_localdir != NULL && ssop_flags & SSOP_CURDIR)
{
if (fputs("tcd ", fd) < 0
! || ses_put_fname(fd, tp->tp_localdir, &ssop_flags) == FAIL
! || put_eol(fd) == FAIL)
goto fail;
did_lcd = TRUE;
}
--- 894,904 ----
// Restore the tab-local working directory if specified
// Do this before the windows, so that the window-local directory can
// override the tab-local directory.
! if ((ssop_flags & SSOP_CURDIR) && tp->tp_localdir != NULL)
{
if (fputs("tcd ", fd) < 0
! || ses_put_fname(fd, tp->tp_localdir, &ssop_flags) == FAIL
! || put_eol(fd) == FAIL)
goto fail;
did_lcd = TRUE;
}
*** ../vim-8.2.4849/src/testdir/test_mksession.vim 2022-03-29 11:56:57.557660671 +0100
--- src/testdir/test_mksession.vim 2022-04-30 15:28:19.823694637 +0100
***************
*** 865,870 ****
--- 865,898 ----
call delete('Xproj', 'rf')
endfunc

+ " Test for saving and restoring the tab-local working directory when there is
+ " only a single tab and 'tabpages' is not in 'sessionoptions'.
+ func Test_mksession_tcd_single_tabs()
+ only | tabonly
+
+ let save_cwd = getcwd()
+ set sessionoptions-=tabpages
+ set sessionoptions+=curdir
+ call mkdir('Xtopdir1')
+ call mkdir('Xtopdir2')
+
+ " There are two tab pages, the current one has local cwd set to 'Xtopdir2'.
+ exec 'tcd ' .. save_cwd .. '/Xtopdir1'
+ tabnew
+ exec 'tcd ' .. save_cwd .. '/Xtopdir2'
+ mksession! Xtest_tcd_single
+
+ source Xtest_tcd_single
+ call assert_equal(2, haslocaldir())
+ call assert_equal('Xtopdir2', fnamemodify(getcwd(-1, 0), ':t'))
+ %bwipe
+
+ set sessionoptions&
+ call chdir(save_cwd)
+ call delete('Xtopdir1', 'rf')
+ call delete('Xtopdir2', 'rf')
+ endfunc
+
" Test for storing the 'lines' and 'columns' settings
func Test_mksession_resize()
mksession! Xtest_mks1.out
*** ../vim-8.2.4849/src/version.c 2022-04-30 15:10:03.100515465 +0100
--- src/version.c 2022-04-30 15:30:52.735530158 +0100
***************
*** 748,749 ****
--- 748,751 ----
{ /* Add new patch number below this line */
+ /**/
+ 4850,
/**/

--
Where do you want to crash today?

/// Bram Moolenaar -- Br...@Moolenaar.net -- http://www.Moolenaar.net \\\
/// \\\
\\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ ///
\\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[Tony Mechelynck]

On Sat, Apr 30, 2022 at 5:12 PM Bram Moolenaar <Br...@moolenaar.net> wrote:
>
>
> Patch 8.2.4850
> Problem: Mksession mixes up "tabpages" and "curdir" arguments.
> Solution: Correct logic for storing tabpage in session. (closes #10312)
> Files: src/session.c, src/testdir/test_mksession.vim

At this patchlevel, I get one warning in Huge, two each (but related)
in Big and Normal, none in Small and Tiny. The following comes from my
Normal build:

gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK
-I/usr/include/gtk-3.0 -I/usr/include/pango-1.0
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
-I/usr/include/harfbuzz -I/usr/include/freetype2
-I/usr/include/libmount -I/usr/include/blkid -I/usr/include/fribidi
-I/usr/include/uuid -I/usr/include/cairo -I/usr/include/pixman-1
-I/usr/include/libpng16 -I/usr/include/gdk-pixbuf-2.0
-I/usr/include/gio-unix-2.0 -I/usr/include/wayland
-I/usr/include/libxkbcommon -I/usr/include/atk-1.0
-I/usr/include/at-spi2-atk/2.0 -I/usr/include/dbus-1.0
-I/usr/lib64/dbus-1.0/include -I/usr/include/at-spi-2.0 -pthread
-O2 -fno-strength-reduce -Wall -Wno-deprecated-declarations
-D_REENTRANT -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -o
objects/session.o session.c
session.c: In function ‘ex_mkrc’:
session.c:982:21: warning: ‘tab_firstwin’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
982 | if (tab_firstwin->w_next != NULL)
| ~~~~~~~~~~~~^~~~~~~~
session.c:627:18: note: ‘tab_firstwin’ was declared here
627 | win_T *tab_firstwin;
| ^~~~~~~~~~~~

The message from my Huge build is (only slightly) different:

gcc -c -I. -Iproto -DHAVE_CONFIG_H -DFEAT_GUI_GTK
-I/usr/include/gtk-3.0 -I/usr/include/pango-1.0
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include
-I/usr/include/harfbuzz -I/usr/include/freetype2
-I/usr/include/libmount -I/usr/include/blkid -I/usr/include/fribidi
-I/usr/include/uuid -I/usr/include/cairo -I/usr/include/pixman-1
-I/usr/include/libpng16 -I/usr/include/gdk-pixbuf-2.0
-I/usr/include/gio-unix-2.0 -I/usr/include/wayland
-I/usr/include/libxkbcommon -I/usr/include/atk-1.0
-I/usr/include/at-spi2-atk/2.0 -I/usr/include/dbus-1.0
-I/usr/lib64/dbus-1.0/include -I/usr/include/at-spi-2.0 -pthread
-O2 -fno-strength-reduce -Wall -Wno-deprecated-declarations
-D_REENTRANT -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1 -o
objects/session.o session.c
session.c: In function ‘makeopens’:
session.c:982:21: warning: ‘tab_firstwin’ may be used uninitialized in
this function [-Wmaybe-uninitialized]
982 | if (tab_firstwin->w_next != NULL)
| ~~~~~~~~~~~~^~~~~~~~


Best regards,
Tony.

[John Marriott]

Hi Tony,

Try this patch.

Cheers
John

[Bram Moolenaar]

The compiler doesn't know that first_tabpage is never NULL.
But the condition is strange anyway, because tab_firstwin changes every
loop and tab_firstwin->w_next may become NULL even though the options
were set. Better set a flag that explicitly says that the option values
need to be restored.

--
From "know your smileys":
:q vi user saying, "How do I get out of this damn emacs editor?"