Fix long standing set_current_item() bug

[Johann Klammer]

Hello,
I've been reading some of the posts here (via gmane), and realized that 
a very recent one describes a problem which I run into about once a year.

I have wrapped up a testcase:
8<---------------------------------------------------------------------------------

#include <stdlib.h>
#include <stdio.h>
#include <ncurses.h>
#include <menu.h>
#include <string.h>
/**
 *\file test3.c
 *test case for set_current_item()
 *should be run in a terminal that's shorter than the menu
 *should show that set_current_item() can leave the menu cursor off-screen
 *
 *compile using:
 *cc test3.c -lncurses -lmenu
 *run using:
 *./a.out
 *in an xterm that is shorter than the menu
 */

#define num_strings 60
char menu_strings[num_strings][10];

void
destroyMenuWnd (MENU * my_menu)
{
  delwin (menu_sub (my_menu));
  delwin (menu_win (my_menu));
}
void
makeMenuWnd (MENU * my_menu,int nitems)
{
  int maxx, maxy;
  WINDOW *menu_win = NULL;
  WINDOW *menu_sub = NULL;
  getmaxyx (stdscr, maxy, maxx);
  menu_win = newwin (maxy, maxx/2, 0, 0);
  menu_sub = derwin (menu_win, maxy-4, maxx/2 - 2, 3, 1);
  set_menu_win (my_menu, menu_win);
  set_menu_sub (my_menu, menu_sub);
  set_menu_format (my_menu, maxy  - 4, 1);
  box (menu_win, 0, 0);
}

void print_menu(MENU *m)
{
        int x=getmaxx(stdscr)/2;
        mvprintw(1,x,"height = %d   ",m->height);
        mvprintw(2,x,"width = %d   ",m->width);
        mvprintw(3,x,"rows = %d   ",m->rows);
        mvprintw(4,x,"cols = %d   ",m->cols);
        mvprintw(5,x,"frows = %d   ",m->frows);
        mvprintw(6,x,"fcols = %d   ",m->fcols);
        mvprintw(7,x,"arows = %d   ",m->arows);
  mvprintw(8,x,"namelen = %d   ",m->namelen);
  mvprintw(9,x,"desclen = %d   ",m->desclen);
  mvprintw(10,x,"marklen = %d   ",m->marklen);
  mvprintw(11,x,"itemlen = %d   ",m->itemlen);
  mvprintw(12,x,"spc_desc = %d   ",m->spc_desc);
  mvprintw(12,x,"spc_cols = %d   ",m->spc_cols);
  mvprintw(13,x,"spc_rows = %d   ",m->spc_rows);
  mvprintw(14,x,"pattern = %s   ",m->pattern);
  mvprintw(15,x,"pindex = %d   ",m->pindex);
  mvprintw(16,x,"win = %p   ",m->win);
  mvprintw(17,x,"sub = %p   ",m->sub);
  mvprintw(18,x,"userwin = %p   ",m->userwin);
  mvprintw(19,x,"usersub = %p   ",m->usersub);
  mvprintw(20,x,"items = %p   ",m->nitems);
  mvprintw(21,x,"nitems = %d   ",m->nitems);
  mvprintw(22,x,"curitem = %p   ",m->curitem);
  mvprintw(23,x,"toprow = %hd   ",m->toprow);
  mvprintw(24,x,"fore = %d   ",m->fore);
  mvprintw(25,x,"back = %d   ",m->back);
  mvprintw(26,x,"grey = %d   ",m->grey);
  mvprintw(27,x,"pad = \'%c\'   ",m->pad);
  mvprintw(28,x,"menuinit = %p   ",m->menuinit);
  mvprintw(29,x,"menuterm = %p   ",m->menuterm);
  mvprintw(30,x,"iteminit = %p   ",m->iteminit);
  mvprintw(31,x,"itemterm = %p   ",m->itemterm);
  mvprintw(32,x,"userptr = %p   ",m->userptr);
  mvprintw(33,x,"mark = %p   ",m->mark);
  mvprintw(34,x,"opt = %d   ",m->opt);
  mvprintw(35,x,"status = %d   ",m->status);
  refresh();
}
/*


m_driver.c:551
        my_top_row = item->y - menu->arows + 1;
    1              53-53+1
nope..
m_global.c:546
_nc_New_TopRow_and_CurrentItem

jump when move from 17 to 16

when the window can be filled by menu...
have to limit toprow to...

rows-frows
or arows? or height? what's te difference?


have to be careful on posting..
looks alright.. after attaching windows but before posting..
*/

static void do_test3(void)
{
  MENU *my_menu;
  int i, maxx, state = 0, idx = 0;
  ITEM **my_items;
  char **pgm_names;
  int lbl_x,maxy;


    state = 0;
//    debugMsg ("allocating items\n");
    my_items = (ITEM **) calloc (num_strings + 1, sizeof (ITEM *));
    for (i = 0; i < num_strings; ++i)
    {
        sprintf(menu_strings[i],"Choice%d",i);
      my_items[i] = new_item (menu_strings[i], menu_strings[i]);
      if (NULL == my_items[i])
      {
//        if (errno == E_BAD_ARGUMENT)
  //        message ("badarg i=%d string=%s\n", i, menu_strings[i]);
    //    if (errno == E_SYSTEM_ERROR)
      //    message ("new_item: syserr i=%d string=%s\n", i, 
menu_strings[i]);
      }
    }
    my_items[num_strings] = (ITEM *) NULL;
    my_menu = new_menu ((ITEM **) my_items);
    set_menu_opts (my_menu, O_ONEVALUE | O_NONCYCLIC | O_ROWMAJOR);
    maxx = getmaxx (stdscr);
    makeMenuWnd (my_menu,num_strings);
    idx = num_strings - 1;
    set_current_item (my_menu, my_items[idx]);//FIXXME
    print_menu(my_menu);
    getch();
    post_menu (my_menu);
    lbl_x = 3 + (maxx/2 - 6) / 2 - strlen ("MENU") / 2;
    mvwprintw (menu_win (my_menu), 1, lbl_x, "%s", "MENU");
    wrefresh (menu_win (my_menu));

//    debugMsg ("starting menu loop\n");
    while (state == 0)
    {
      int c;
      ITEM *selection;
      c = getch ();
      switch (c)
      {
      case 263:                //ESC
      case 27:                 //Backspace
        state = -1;
        break;
      case KEY_DOWN:
        menu_driver (my_menu, REQ_DOWN_ITEM);
        break;
      case KEY_UP:
        menu_driver (my_menu, REQ_UP_ITEM);
        break;
      default:
        break;
      }
      print_menu(my_menu);
      wrefresh (menu_win (my_menu));
    }
    unpost_menu (my_menu);
    destroyMenuWnd (my_menu);
    free_menu (my_menu);
    for (i = 0; i < num_strings; ++i)
    {
      free_item (my_items[i]);
    }
    free (my_items);

}

int main(int argc, char *argv[])
{

  initscr ();
  start_color ();
  cbreak ();
  noecho ();
  nonl ();
  keypad (stdscr, TRUE);
  refresh ();
  do_test3();

  endwin ();
  return 0;
}



8<---------------------------------------------------------------------------------



And a patch:


8<---------------------------------------------------------------------------------

--- ./ncurses_prev/menu/m_global.c      2012-06-10 02:09:15.000000000 +0200
+++ ./ncurses_now/menu/m_global.c       2014-03-14 12:15:38.000000000 +0100
@@ -529,6 +529,8 @@ _nc_Show_Menu(const MENU * menu)
     }
 }

+#define minimum(a,b) ((a)<(b) ? (a): (b))
+

/*---------------------------------------------------------------------------
 |   Facility      :  libnmenu
 |   Function      :  void _nc_New_TopRow_and_CurrentItem(
@@ -568,7 +570,7 @@ _nc_New_TopRow_and_CurrentItem(

       cur_item = menu->curitem;
       assert(cur_item);
-      menu->toprow = (short)new_toprow;
+      menu->toprow = (short)/*new_toprow*/ 
((menu->rows-menu->frows)>=0)?minimum(menu->rows-menu->frows,new_toprow):0;
       menu->curitem = new_current_item;

       if (mterm_called)
@@ -590,7 +592,7 @@ _nc_New_TopRow_and_CurrentItem(
     }
   else
     {                          /* if we are not posted, this is quite simple */
-      menu->toprow = (short)new_toprow;
+      menu->toprow = (short) 
((menu->rows-menu->frows)>=0)?minimum(menu->rows-menu->frows,new_toprow):0;//does 
this work when not posted? new_toprow;
       menu->curitem = new_current_item;
     }
 }

8<---------------------------------------------------------------------------------
(it is against the debian sauce, so may need editing)