[comp.windows.open-look] XView scrolling list bug?

gw18@prism.gatech.EDU (Greg Williams) (03/01/91)

The following program produces an intermitant "Memory fault" crash.  Its
occuring sometime before the routine "dir_proc" is called.  The crash is
somewhere inside "xv_get".  It takes a few minutes of playing and changing
directories to get the initial crash, but after it crashes once, I can
usually reproduce the crash, but not every time.  I've run this code on a
4/65 and a 4/490, and get the same error.  I'm running Open Windows as
supplied by Sun.  Can anyone tell me if I'm doing something wrong with
the XView code?  The crash occurs too often to ignore it.  I appreciate any
and all help I get.

----code begins here----

/*
 * This program creates two scrolling lists, one with directories and one
 * with files.  It allows the user to search through directories for files
 * and then select a file to "open".
 */

#include <stdio.h>
#include <stdlib.h>
#include <xview/xview.h>
#include <xview/panel.h>
#include <dirent.h>
#include <sys/stat.h>
#include <sys/types.h>

#define DESELECT 0
#define SELECT 1
#define ENTRY_KEY 5877
#define NULL_STR ""

Frame frame;
Panel panel;
Panel_item pi_dir, pi_file, ti_file, mi_dir, bi_open;
DIR *dirp;
struct dirent *entry;
char *current;

int num_files, num_dirs;

int Open();
int Cancel();
void setup_list();
void dir_proc();
void file_proc();
int check_read();

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

{
  char *tmpname;

/*
 * Start in the HOME directory
 */

  tmpname = getenv("HOME");
  current = (char *)malloc(strlen(tmpname)+1);
  memset(current, '\0', strlen(tmpname)+1);
  strcat(current, tmpname);
  free(tmpname);

  xv_init(XV_INIT_ARGC_PTR_ARGV, &argc, argv, NULL);

  frame = (Frame)xv_create(NULL, FRAME,
    FRAME_LABEL,		"File Open",
    NULL);

  panel = (Panel)xv_create(frame, PANEL, NULL);

  mi_dir = (Panel_item)xv_create(panel, PANEL_MESSAGE,
    PANEL_LABEL_STRING,		current,
    PANEL_LABEL_BOLD,		TRUE,
    NULL);

  ti_file = (Panel_item)xv_create(panel, PANEL_TEXT,
    PANEL_LABEL_STRING, 	"File:",
    PANEL_VALUE_DISPLAY_LENGTH,	50,
    PANEL_VALUE_STORED_LENGTH,	256,
    PANEL_NEXT_ROW,		-1,
    PANEL_VALUE,		NULL_STR,
    NULL);

  pi_dir = (Panel_item)xv_create(panel, PANEL_LIST,
    PANEL_LABEL_STRING,		"Directories:",
    PANEL_LIST_DISPLAY_ROWS,	10,
    PANEL_CHOOSE_ONE,		TRUE,
    PANEL_CHOOSE_NONE,          TRUE,
    PANEL_READ_ONLY,		TRUE,
    PANEL_NOTIFY_PROC,		dir_proc,
    PANEL_NEXT_ROW,		-1,
    PANEL_LIST_WIDTH,		150,
    PANEL_LAYOUT,		PANEL_VERTICAL,
    NULL);

  pi_file = (Panel_item)xv_create(panel, PANEL_LIST,
    PANEL_LABEL_STRING,		"Files:",
    PANEL_LIST_DISPLAY_ROWS,	10,
    PANEL_CHOOSE_ONE,		TRUE,
    PANEL_CHOOSE_NONE,		TRUE,
    PANEL_READ_ONLY,		TRUE,
    PANEL_NOTIFY_PROC,		file_proc,
    PANEL_LIST_WIDTH,		150,
    PANEL_LAYOUT,		PANEL_VERTICAL,
    NULL);

  bi_open = (Panel_item)xv_create(panel, PANEL_BUTTON,
    PANEL_LABEL_STRING,		"Open",
    PANEL_NOTIFY_PROC,		Open,
    XV_X,			370,
    XV_Y,			73,
    NULL);

  (void)xv_create(panel, PANEL_BUTTON,
    PANEL_LABEL_STRING,		"Cancel",
    PANEL_NOTIFY_PROC,		Cancel,
    XV_X,			367,
    XV_Y,			103,
    NULL);

  if (check_read(current))
    setup_list();

  window_fit(panel);
  window_fit(frame);

  xv_main_loop(frame);
}


/*
 * Re-read the directory entries for the scrolling lists
 */

void setup_list()

{
  char *tmpname;
  int i;

  xv_set(ti_file,
    PANEL_VALUE,	NULL_STR,
    NULL);

  xv_set(bi_open,
    XV_SHOW, FALSE,
    NULL);

  xv_set(mi_dir,
    PANEL_LABEL_STRING,	current,
    NULL);

/*
 * Hide the scroll lists while re-computing the lists.  Saves on re-draw
 * time.
 */

  xv_set(pi_dir,
    XV_SHOW,		FALSE,
    NULL);

  xv_set(pi_file,
    XV_SHOW,		FALSE,
    NULL);

/*
 * First delete the old entries
 */

  for (i = num_dirs - 1; i >= 0; i--)
    xv_set(pi_dir, PANEL_LIST_DELETE, i, NULL);

  for (i = num_files - 1; i >= 0; i--)
    xv_set(pi_file, PANEL_LIST_DELETE, i, NULL);

  num_files = 0;
  num_dirs = 0;

/*
 * Now read the new entries
 */

  dirp = opendir(current);

  while ((entry = readdir(dirp)) != NULL) {
    tmpname = (char *)malloc(strlen(current) + entry->d_namlen + 2);
    memset(tmpname, '\0', strlen(current) + entry->d_namlen + 2);
    strcat(tmpname, current);
    strcat(tmpname, "/");
    strcat(tmpname, entry->d_name);
    if (dir_test(tmpname)) {

/*
 * A directory
 */

      if (strcmp(entry->d_name, ".")) {
        xv_set(pi_dir,
          PANEL_LIST_INSERT,	num_dirs,
          PANEL_LIST_STRING,	num_dirs, entry->d_name,
          NULL);
        num_dirs++;
      }
    }
    else {

/*
 * A file
 */

      if (strncmp(entry->d_name, ".", 1)) {
        xv_set(pi_file,
	  PANEL_LIST_INSERT,	num_files,
	  PANEL_LIST_STRING,	num_files, entry->d_name,
	  NULL);
        num_files++;
      }
    }
    free(tmpname);
  }

/*
 * Done, show the new scroll panels
 */

  xv_set(pi_dir,
    XV_SHOW,		TRUE,
    NULL);
  xv_set(pi_file,
    XV_SHOW,		TRUE,
    NULL);

  closedir(dirp);
}


/*
 * Handle the selection of a directory entry in the scroll list
 */

void dir_proc(item, string, client_data, op, event)
Panel_item item;
char *string;
caddr_t client_data;
Panel_list_op op;
Event *event;

{
  char *oldcurrent;
  int i;

/*
 * Make sure the user selected and didn't drag.  Dragging may cause unwanted
 * changes
 */

  if (op == SELECT && event_id(event) != LOC_DRAG &&

/*
 * Make sure we can read the directory before making any changes
 */

      check_read(current, string)) {

      oldcurrent = (char *)malloc(strlen(current)+1);
      memset(oldcurrent, '\0', strlen(current)+1);
      strcat(oldcurrent, current);
      free(current);

      if (strcmp(string, "..")) {

/*
 * Add the new directory to the "current" directory string
 */

        current = (char *)malloc(strlen(oldcurrent) + strlen(string) + 2);
        memset(current, '\0', strlen(oldcurrent) + strlen(string) + 2);
        strcat(current, oldcurrent);
        if (strlen(oldcurrent) > 1)
          strcat(current, "/");
        strcat(current, string);
      }
      else {

/*
 * Go back one directory
 */

        i = strlen(oldcurrent);

        while(oldcurrent[i] != '/') {
	  oldcurrent[i] = '\0';
	  i--;
        }

/*
 * Tried to go back from the root directory?
 */

        if (i < 1) {
	  oldcurrent[0] = '/';
	  oldcurrent[1] = '\0';
	  i = 1;
        }
        else
          oldcurrent[i] = '\0';

        current = (char *)malloc(i);
        memset(current, '\0', i);
        strcat(current, oldcurrent);
      }

      free(oldcurrent);

      setup_list();
  }
}


/*
 * Handle a selection on the file list
 */

void file_proc(item, string, client_data, op, event)
Panel_item item;
char *string;
caddr_t client_data;
Panel_list_op op;
Event *event;

{
  if (op == SELECT) {
    xv_set(ti_file,
      PANEL_VALUE,	string,
      NULL);
    xv_set(bi_open,
      XV_SHOW,		TRUE,
      NULL);
  }
  else if (op == DESELECT) {
    xv_set(ti_file,
      PANEL_VALUE,	NULL_STR,
      NULL);
    xv_set(bi_open,
      XV_SHOW,		FALSE,
      NULL);
  }
}


/*
 * Acknowledge an Open button press
 */

int Open(item, event)
Panel_item item;
Event *event;

{
  if (strcmp(NULL_STR, (char *)xv_get(ti_file, PANEL_VALUE))) {
    printf("Open: %s\n", xv_get(ti_file, PANEL_VALUE));
    xv_destroy_safe(frame);
  }
}

int Cancel(item, event)
Panel_item item;
Event *event;

{
  xv_destroy_safe(frame);
}


/*
 * This routine tests to see if a file name is a directory entry
 */

dir_test(filename)
char *filename;

{
  struct stat statbuf;

  stat(filename, &statbuf);
  return(S_ISDIR(statbuf.st_mode));
}


/*
 * Check to see if the directory is readable
 */

int check_read(oldpath, newdir)
char *oldpath, *newdir;

{
  char *both;
  DIR *test_open;

  both = (char *)malloc(strlen(oldpath) + strlen(newdir) + 2);
  memset(both, '\0', strlen(oldpath) + strlen(newdir) + 2);
  strcat(both, oldpath);
  strcat(both, "/");
  strcat(both, newdir);

  if (!(test_open = opendir(both))) {
    free(both);
    return 0;
  }
  else {
    free(both);
    closedir(test_open);
    return 1;
  }
}

----code ends here----

murph@qualix.com (Joe Murphy, at large) (03/02/91)

In article <23080@hydra.gatech.EDU> gw18@prism.gatech.EDU (Greg Williams) writes:
>
>The following program produces an intermitant "Memory fault" crash.  Its
>occuring sometime before the routine "dir_proc" is called.  The crash is
>somewhere inside "xv_get".  It takes a few minutes of playing and changing
>directories to get the initial crash, but after it crashes once, I can
>usually reproduce the crash, but not every time.  I've run this code on a
>4/65 and a 4/490, and get the same error.  I'm running Open Windows as
>supplied by Sun.  Can anyone tell me if I'm doing something wrong with
>the XView code?  The crash occurs too often to ignore it.  I appreciate any
>and all help I get.
>

I'm pretty sure you are hitting the same bug I did involving scrolling
lists in the Xview2 library.  Here are the relevant clues:

>----code begins here----
>
>/*
> * This program creates two scrolling lists, one with directories and one
> * with files.  It allows the user to search through directories for files
> * and then select a file to "open".
> */
>
>	...
>
>  pi_dir = (Panel_item)xv_create(panel, PANEL_LIST,
>	...
>    PANEL_CHOOSE_ONE,		TRUE,
>	...
>    NULL);
>
>	...
>/*
> * First delete the old entries
> */
>
>  for (i = num_dirs - 1; i >= 0; i--)
>    xv_set(pi_dir, PANEL_LIST_DELETE, i, NULL);
>
>  for (i = num_files - 1; i >= 0; i--)
>    xv_set(pi_file, PANEL_LIST_DELETE, i, NULL);
>
>	...
>----code ends here----


There is the bug in the xview2 library (source file is
xview2/lib/libxvol/panel/p_list.c) where the pointer dp->current_row
is left pointing to garbage if you delete the selected item,
and blows up sooner or later when the xview code later attempts
to "deselect" that vanished row. 

The bug doesn't show up for "non-exclusive" lists (ie
PANEL_CHOOSE_ONE,  FALSE,) so that is a possible work-around.  
For a while I had a work-around that involved forcing a selection
of an item not being deleted before the PANEL_LIST_DELETE, but
clearly this did not work when the whole list is being cleared
as in your case.  Finally I cobbled together some code where 
I clear out the pointer after every PANEL_LIST_DELETE, and have
not seen any problems since then.  I could have course modified 
the library directly, but I don't want to ship my applications with
the Xview2.0 libraries static linked (otherwise they end up being
megabytes in size instead of a couple of hundred Kbytes altogether
 - ie 1 floppy instead of many). 

BTW I strongly recommend you get xview2 library src from somewhere
(I got mine from uunet) and build it with "-g" turned on so you can
follow core dumps into the buggy parts of the library.

Here are the relevant parts of "p_list.c":

**********************************
xview2/lib/libxvol/panel/p_list.c
**********************************
	...

Pkg_private     Xv_opaque
panel_list_set_avlist(panel_list_public, avlist)
    Panel_item      panel_list_public;
    Attr_avlist     avlist;
{
    ...

    /*
     * Process the attributes that rely on the already processed attributes.
     */
    avlist = orig_avlist;
    for (avlist = orig_avlist; *avlist; avlist = attr_next(avlist)) {
	switch ((int) avlist[0]) {

	  ...

	  case PANEL_LIST_DELETE:{
		int             which_row = (int) avlist[1];
		Row_info       *node = dp->rows;

		while (node && (node->row != which_row))
		    node = node->next;
		if (node)
		    panel_list_delete_row(dp, node, DO_NOT_REPAINT_LIST);
		break;
	    }

	  ...

	}
    }

    /* Non-exclusive lists can have no current row */
    if (!dp->choose_one)
	dp->choose_none = TRUE;

    if (!dp->choose_none && dp->rows && !dp->setting_current_row) {
        /* If no row is selected, then select the first row */
        for (row = dp->rows; row; row = row->next)
            if (row->selected)
            	break;
        if (!row) {
    	    dp->current_row = dp->rows;
            dp->current_row->selected = TRUE;
        }
    }

    ...

    return XV_OK;
}


static void
set_current_row(dp, event_row, event)
    Panel_list_info	*dp;
    Row_info		*event_row;
    Event		*event;	/* NULL => not event generated */
{
    int		new_state = TRUE;
    int		toggle = FALSE;
    
    if (dp->choose_one) {
    	if (dp->current_row == event_row) {
    	    if (dp->choose_none)
    	    	toggle = TRUE;
    	} else if (dp->current_row) {
	    dp->setting_current_row = TRUE;
	    dp->current_row->selected = FALSE;
vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
    /* usually detonates below here, since dp->current_row can be trash */
    	    show_feedback(dp, dp->current_row, event);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    	}
	if (toggle)
	    new_state = event_row->selected ? FALSE : TRUE;
	event_row->selected = new_state;
	show_feedback(dp, event_row, event);
	dp->setting_current_row = FALSE;
    } else {
    	new_state = event_row->selected ? FALSE : TRUE;
    	event_row->selected = new_state;
    	show_feedback(dp, event_row, event);
    }
    dp->current_row = event_row;
}

blanchar@prlhp1.prl.philips.co.uk (blanchar) (03/04/91)

In article <23080@hydra.gatech.EDU>, gw18@prism.gatech.EDU (Greg Williams) writes:
> 
> The following program produces an intermitant "Memory fault" crash.  Its
> occuring sometime before the routine "dir_proc" is called.  The crash is
> somewhere inside "xv_get".  It takes a few minutes of playing and changing
> directories to get the initial crash, but after it crashes once, I can
> usually reproduce the crash, but not every time.  I've run this code on a
> 4/65 and a 4/490, and get the same error.  I'm running Open Windows as
> supplied by Sun.  Can anyone tell me if I'm doing something wrong with
> the XView code?  The crash occurs too often to ignore it.  I appreciate any
> and all help I get.
> 
> ----code begins here----

I did a very similar thing but with only one scrolling list and had the
same problem.
This is where the problem is I think.....

> /*
>  * Hide the scroll lists while re-computing the lists.  Saves on re-draw
>  * time.
>  */
> 
>   xv_set(pi_dir,
>     XV_SHOW,		FALSE,
>     NULL);
> 
>   xv_set(pi_file,
>     XV_SHOW,		FALSE,
>     NULL);
> 
ETC..

What I did was this.

xv_set(list,XV_SHOW,FALSE,NULL);
xv_destroy_safe(list);
create_list(list);
/* create_list() is easy if you're using guide since it generates
functions  to create all it's objects and all you have to do is call 
the correct one */
xv_set(list,XV_SHOW,FALSE,NULL);
/* this last XV_SHOW,FALSE is here to make sure it doesn't show the
list until it is completed */
Then add the entries to the list as before. You don't need the
PANEL_LIST_INSERT if it's going at the end.
xv_set(list,XV_SHOW,TRUE,NULL);

I hope this helps. It worked for me!

Simon Blanchard