fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
Most people are aware that HP is working on a widget library, one version of which will be put into the public domain. This library will initially be R2 based. Like others trying to use the R2 intrinsics we have encountered significant problems which we have needed to fix in order to proceed with our development. The bugs, and usually a proposed fix, have been reported to MIT. MIT has blessed some of the fixes but has simply not had the time to respond to all of the bugs. My guess is that MIT has shifted its focus to the upcoming (Oct) R3 tape release and that they are spending most of their cycles trying to get it right. HP supports the Xt Intrinsics as an integral part of an X Windows User Interface development tool. In that spirit we are re-posting the unresolved R2 Intrinsics bugs reported by HP as well as our fixes to these bugs. <<AGAIN, THESE ARE HP, NOT MIT FIXES>> Also, within the next 2 weeks we will be donating our widget library to MIT. As part of that package we will make available the R2 intrinsics with all patches (HP's and MIT's). --Bob Miller
fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
VERSION:
Xtk release 2
SYNOPSIS:
CoreDestroy() should free up the memory occupied by the
widget's destroy callback list.
DESCRIPTION:
The CoreDestroy() routine fails to free up the memory occupied
by a widget's destroy callback list, thus cause a memory leak.
REPEAT BY:
Write a program which creates a bunch of widgets, and then adds
destroy callbacks to each widget; then destroy the widgets.
(Have the program repeat this several times).
FIX:
/* The following is a patch to Core.c */
291a300,303
>
> /* Remove the destroy_callbacks list */
> XtRemoveAllCallbacks(widget, XtNdestroyCallback);
> fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
VERSION:
Xtk release 2
SYNOPSIS:
AddForwardingHandler() does not take window borders into
consideration when checking if cursor is in a window.
DESCRIPTION:
AddForwardingHandler(), which is part of the input focus
mechanism within Xtk, does not take window borders into
consideration when it checks to see if the cursor is already
in the window to which the focus is being set. This causes
the widget to not get the initial FocusIn event, which is also
generated by AddForwardingHandler().
FIX:
/* The following is a patch to Event.c */
997a998
> int ul_x, lr_x, ul_y, lr_y;
1047,1048c1048,1063
< if (win_x >= 0 && win_x < widget->core.width &&
< win_y >= 0 && win_y < widget->core.height) {
---
>
> /*
> * The work variables are necessary because
> * the core's height and width fields are unsigned, and win_x and
> * win_y may be negative; if they are, then the comparison can fail
> * when it shouldn't. Additionally, the widget's borders need to
> * be considered in this calculation.
> */
> ul_x = (int)(0 - widget->core.border_width);
> ul_y = (int)(0 - widget->core.border_width);
> lr_x = (int)(widget->core.width + (widget->core.border_width << 1));
> lr_y = (int)(widget->core.height + (widget->core.border_width << 1));
>
> if ((win_x >= ul_x) && (win_x < lr_x) &&
> (win_y >= ul_y) && (win_y < lr_y))
> {fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
VERSION:
Xtk release 2
SYNOPSIS:
Using XtParseTranslationTable() causes a program to grow without
an upper bound.
DESCRIPTION:
XtParseTranslationTable() returns a pointer to a translation
state table, which in turn contains potentially many other
pointers to blocks of memory (state records, action records, etc).
Unfortunately, the toolkit does not provide a mechanism for
freeing up this memory once an application is done with it.
It appears that the reason the memory is not freed up is because
the toolkit caches these parsed translation table, thus
potentially saving time (and space) if the same translation
is ever parsed again. However, our widget set parses many
(possibly 100 or more, depending upon what widgets you use),
and each of the translation tables are unique.
If an application, such as a window manager, is creating and
destroying alot of widgets, it's size continues to grow because
all of these state tables are cached, but never used again after
the associated widget is destroyed.
The patches which follow affect the following files:
1) TMstate.c - Added XtDestroyStateTable(), which frees up
a translation state table and all underlying
malloc memory.
Modified MergeStates() to create new copies
of the parameter strings and array of string
pointers, rather than just copying the existing
pointers.
Removed the caching of the parsed event state
tables in XtOverrideTranslations and
XtAugmentTranslations.
Added a call to XtDestroyStateTable() in both
XtOverrideTranslations and XtAugmentTranslations,
to free up the widgets old translation state table.
2) TMparse.c - Removed the caching of the parsed event state
table in XtParseTranslationTable().
3) Core.c - Added a call to XtDestroyStateTable() in
CoreDestroy(), to free up the widgets state
table.
REPEAT-BY:
Have a program which parses several unique translation tables,
and watch the size of the program grow.
FIX:
/* This is a patch for Core.c */
279a280,287
>
>
> /* Destroy the translation state table */
>
> XtDestroyStateTable (widget->core.widget_class,
> widget->core.tm.translations);
>
>
/* This is a patch for TMparse.c */
1420a1421
> int noArgs = 0;
1423a1425,1428
> to.addr = NULL;
> to.size = 0;
> _CompileTranslations (NULL, &noArgs, &from, &to);
> /*
1425a1431
> */
/* This is a patch for TMparse.c */
865a866
> register int i;
905c906,908
< a->params = b->params;
---
>
>
> /* a->params = b->params; */
906a910,919
> if (a->num_params > 0)
> a->params = (char **) XtMalloc (sizeof (char *) * b->num_params);
> else
> a->params = NULL;
> for (i = 0; i < b->num_params; i++)
> {
> a->params[i] = (char *) XtMalloc (strlen (b->params[i]) + 1);
> strcpy (a->params[i], b->params[i]);
> }
>
1082a1096
> int noArgs = 0;
1092a1107,1108
> _MergeTranslations (NULL, &noArgs, &from, &to);
> /*
1094a1111
> */
1095a1113,1119
>
> /* Destroy the translation state table */
>
> XtDestroyStateTable (widget->core.widget_class,
> widget->core.tm.translations);
>
>
1136a1161
> int noArgs = 0;
1146a1172,1173
> _MergeTranslations (NULL, &noArgs, &from, &to);
> /*
1148a1176
> */
1149a1178,1184
>
> /* Destroy the translation state table */
>
> XtDestroyStateTable (widget->core.widget_class,
> widget->core.tm.translations);
>
>
1417a1453,1494
>
>
> /* HP added translation state table deallocation procedure. */
> /* This is used by both toolkit functions and widgets to */
> /* eliminate a huge memory whole. */
>
> XtDestroyStateTable (class, stateTable)
>
> WidgetClass class;
> XtTranslations stateTable;
>
> {
> register StatePtr state, nextState;
> register ActionPtr action, nextAction;
>
> /* Don't do anything if the state table is the widget class's state table */
> if (stateTable && (stateTable != (XtTranslations)class->core_class.tm_table))
> {
> /* Free up each state record and all associated action records */
> for (state = stateTable->head; state != NULL; )
> {
> for (action = state->actions; action != NULL; )
> {
> nextAction = action->next;
> FreeActions(action);
> action = nextAction;
> }
>
> nextState = state->forw;
> XtFree(state);
> state = nextState;
> }
>
> /* Free up quark Table, event object array and the state table */
> XtFree(stateTable->quarkTable);
> XtFree(stateTable->eventObjTbl);
> XtFree(stateTable);
> }
> }
>
>
> fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
### bug number: 649
### area: Xt
### severity: low
### comments:
VERSION:
X Window System, Version 11, Release 2
AREA:
Xt
SYNOPSIS:
XtNameToWidget fails on primitives with popup children
DESCRIPTION:
The XtNameToWidget function that translates a widget name
to a widget instance doesn't take into account a primitive
widget with popup childrens. This is used, for instance, with
the HP Widget set when using their menu manager, menu panels, etc...
Is the "if (!XtIsComposite(root)) return NULL;" to strong in
NameListToWidget?
FIX:
/* Patch to Intrinsic.c */
252d251
< if (! XtIsComposite(root)) return NULL;
254,257c253,260
< children = ((CompositeWidget) root)->composite.children;
< for (i = 0; i < ((CompositeWidget) root)->composite.num_children; i++) {
< if (name == children[i]->core.xrm_name)
< return NameListToWidget(children[i], &names[1]);
---
> /* Check normal children only if this is a composite widget */
> if (XtIsComposite(root))
> {
> children = ((CompositeWidget) root)->composite.children;
> for (i = 0; i < ((CompositeWidget) root)->composite.num_children; i++) {
> if (name == children[i]->core.xrm_name)
> return NameListToWidget(children[i], &names[1]);
> }
258a262,263
>
> /* Even non-composite widgets may have popup children */fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
### bug number: 363
### area: Xt
### severity:
### assigned to: swick
### status: open
### comments:
VERSION:
Xtk release 2
SYNOPSIS:
Adding a global translation to a realized widget destroys the
widget's translations.
DESCRIPTION:
As I reported in an earlier bug report, if a new translation is
added to a realized widget (using XtOverrideTranslation), and the
action to which the translation is tied has not been previously used
for that widget, all of the translations for that widget get
destroyed.
REPEAT-BY:
Register a global action routine, and set up a translation for a
realized widget which uses this action.
FIX:
There were actually several problems in the translation
manager which contributed to this problem. They include:
1) After the first time a widget's translations were bound, the
proc_table size was never again changed; even if future translations
referenced different action routines.
2) After an augment or override request, the translations were not
really being rebound; this is a result of the table being the
wrong size and the fact that only NULL table entries are bound,
and none of the entries had been NULL'ed out.
3) The old event handler for the widget was not removed.
4) The new translations were never installed.
*************************** diffs start here *******************************
/* Patch to TMstate.c */
727a728
>
1078,1081c1079
< /*
< MergeTables(widget->core.translations, new, TRUE);
< */
< Cardinal numQuarks =0;
---
> Cardinal numQuarks = 0;
1083a1082
>
1089c1088,1089
< if (widget->core.tm.translations != NULL)
---
>
> if (widget->core.tm.translations != NULL)
1090a1091
>
1093,1097c1094,1126
< /* _XtOverrideTranslations(widget->core.tm.translations, new);*/
< widget->core.tm.translations =(*(XtTranslations*)to.addr);
< if (XtIsRealized(widget))
< _XtBindActions(widget,&widget->core.tm,numQuarks);
<
---
>
> widget->core.tm.translations =(*(XtTranslations*)to.addr);
>
> if (XtIsRealized(widget))
> {
> int i;
>
> if (numQuarks != widget->core.tm.translations->numQuarks)
> {
> widget->core.tm.proc_table= (XtActionProc*) XtRealloc(
> widget->core.tm.proc_table,
> widget->core.tm.translations->numQuarks * sizeof(XtActionProc));
> }
>
> /*
> * Since the old table was merged into the new table,
> * we need to force all translations to be rebound; this
> * is because the indices into the proc_table have changed,
> * and duplicate actions will have been removed by MergeTables().
> */
> for (i=0; i < widget->core.tm.translations->numQuarks; i++)
> widget->core.tm.proc_table[i] = 0;
>
> /*
> * Bind the new actions (rebind the old actions); we must
> * also remove the old event handler, and then install the
> * new set of translations.
> */
> _XtBindActions(widget, &widget->core.tm, 0);
> XtRemoveEventHandler (widget, XtAllEvents, True, _XtTranslateEvent,
> (caddr_t)&widget->core.tm);
> _XtInstallTranslations (widget, widget->core.tm.translations);
> }
1104c1133
< Cardinal numQuarks =0;
---
> Cardinal numQuarks = 0;
1106a1136
>
1112c1142,1143
< if (widget->core.tm.translations != NULL)
---
>
> if (widget->core.tm.translations != NULL)
1113a1145
>
1116,1119d1147
< /* _XtAugmentTranslations(widget->core.tm.translations, new);*/
< widget->core.tm.translations = (*(XtTranslations*)to.addr);
< if (XtIsRealized(widget))
< _XtBindActions(widget,&widget->core.tm,numQuarks);
1120a1149,1180
> widget->core.tm.translations = (*(XtTranslations*)to.addr);
>
> if (XtIsRealized(widget))
> {
> int i;
>
> if (numQuarks != widget->core.tm.translations->numQuarks)
> {
> widget->core.tm.proc_table= (XtActionProc*) XtRealloc(
> widget->core.tm.proc_table,
> widget->core.tm.translations->numQuarks * sizeof(XtActionProc));
> }
>
> /* Force all entries to be bound (see note below) */
> for (i = 0; i < widget->core.tm.translations->numQuarks; i++)
> widget->core.tm.proc_table[i] = 0;
>
> /*
> * Bind the new actions and all existing actions; we must rebind
> * the old actions because MergeTables() may have compressed the
> * proc_table, and we thus no longer know where the new entries
> * start (This really is caused by a bug in ParseActionSeq(),
> * which doesn't check to see if an action already has an entry
> * in the proc_table; thus, an action can initially appear more
> * than once). We must also remove the old event handler,
> * and then install the new set of translations.
> */
> _XtBindActions(widget, &widget->core.tm, 0);
> XtRemoveEventHandler (widget, XtAllEvents, True, _XtTranslateEvent,
> (caddr_t)&widget->core.tm);
> _XtInstallTranslations (widget, widget->core.tm.translations);
> }fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
### bug number: 549
### area: Xt
### severity: low
### comments:
VERSION:
Xtk Release 2
SYNOPSIS:
Xtk will not allow a widget (during its phase 2 destroy) to destroy
another, unrelated, widget.
DESCRIPTION:
As XtNextEvent() currently implements the second phase of the destory
process, it is not possible for a widget being destroyed to in turn
initiate a destroy on another, unrelated widget (using XtDestroyWidget()).
The problem arises because XtNextEvent() takes the current DestroyList,
performs all of the callbacks, and then NULLs out the DestroyList; any
new entries on the DestroyList are lost.
REPEAT-BY:
Destroy a widget whose Destroy routine then attempts to destroy another
widget.
FIX:
Changing XtNextEvent() to perform the following, will fix the bug:
while (DestroyList != NULL)
{
CallbackList currentList = DestroyList;
DestroyList = NULL;
_XtCallCallbacks (¤tList, (caddr_t), NULL);
_XtRemoveAllCallbacks (¤tList);
}fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
### bug number: 420
### area: Xt
### severity: medium
### comments: workaround: don't reuse these resources
VERSION:
Xtk release 2
SYNOPSIS:
CoreDestroy() frees up resources owned by the application
DESCRIPTION:
When a widget is destroyed, the CoreDestroy() routine frees up
the background pixmap and the border pixmap, if they are defined.
Unfortunately, these resource were owned by the application; the
intrinsics had not ever made their own private copy. The next
time the application tries to use that pixmap, an X Error occurs.
REPEAT-BY:
Create a widget and assign a background pixmap.
Destroy the widget.
Create a second widget using the same background pixmap.
An X Error occurs.
FIX:
/* Patch to Core.c */
263a264,265
> /*
> DON'T FREE UP RESOURCES WE DID NOT ALLOCATE!!!!
267a270
> */fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
### bug number: 518
### area: Xt
### severity: low
### comments:
VERSION:
Xtk release 2
SYNOPSIS:
Under certain conditions, it is possible for a widget to have
more than one keyboard focus entry on the grablist, when in fact
only one should have been present.
DESCRIPTION:
The current implementation of the input focus for Xtk enforces the
rule that if a widget has a non-focus entry on the grab list, then
a focus entry for that widget will always be placed after the
non-focus entry. Unfortunately, there are bugs in the current
implementation, which may cause this rule to be broken. Additionally,
the current implementation looks on the grab list for an existing
focus entry before adding a new one; if an existing one is found,
then it is replaced - you don't end up with duplicate focus entries
for a widget; again, there is a bug which causes duplicate entries
to be placed on the grablist.
When a focus grab is to be added to the grablist, the current
implementation searches the grab list to see if there is already
a grab entry for this widget. If there is, and the current entry
is a non-focus entry, then it simply adds the focus entry after
the non-focus entry; unfortunately, if there was already a focus
entry after the non-focus entry, you now end up with 2 (or more)
focus entries on the grab list. At this point, the grablist has
suffered irreparable damage.
An additional problem can occur if a widget has multiple non-focus
entries on the grab list. To guarantee that the focus entry is
always after the non-focus entries, the grablist must be searched
until all non-focus grab list entries have been found for this
widget; the focus entry must then be placed after the last non-focus
entry.
FIX:
/* Patch to Event.c */
/* add a grab record to the list, or replace the focus widget in an
existing grab record. Returns True if the action was not a no-op.
*/
static Boolean InsertKeyboardGrab(widget, keyboard_focus)
Widget widget;
Widget keyboard_focus;
{
register GrabRec *gl;
register Widget w;
GrabRec* ge;
Boolean found = False;
GrabRec * lastMatch = NULL;
if (grabList == NULL) {
AddGrab( widget, False, False, keyboard_focus );
return True;
}
/* look for a keyboard grab entry for the same parent; if none,
reposition this entry after any other entries for the same tree */
for (w = widget; (w != NULL) && !((w != widget) && (lastMatch != NULL));
w = w->core.parent)
{
for (gl = grabList; gl != NULL; gl = gl->next)
{
if (gl->widget == w)
{
if (gl->widget == widget)
{
/*
* Because we know that focus entries always appear
* after a normal grab entry for the same widget, we
* can't stop when we encounter the normal grab entry;
* we must continue checking until we either find a
* matching focus entry, or we find no more entries for
* this widget.
*/
if (gl->keyboard_focus != NULL)
{
/*
* Since the widget in the grab entry matches the
* passed-in widget, and since we know that a non-focus
* entry always appears after a normal grab list entry,
* we are assured that this is the only focus entry for
* this widget, so we can update it and then return.
*/
if (gl->keyboard_focus == keyboard_focus)
{
return False;
}
else
{
SendFocusNotify(gl->keyboard_focus, FocusOut);
gl->keyboard_focus = keyboard_focus;
return True;
}
}
else
{
/*
* Keep going, in case there is a focus entry
* following this one.
*/
lastMatch = gl;
found = True;
}
}
else
{
/*
* One of our parent widgets has an entry on the
* grab list; add our entry after its entry.
*/
AddGrab( widget, False, False, keyboard_focus );
ge = grabList;
grabList = grabList->next;
ge->next = gl->next;
gl->next = ge;
return True;
}
}
}
}
if (found)
{
/*
* The specified widget had a non-focus entry on the grab
* list, but currently no focus grab. Therefore, add the
* focus entry after the non-focus entry.
*/
AddGrab( widget, False, False, keyboard_focus );
ge = grabList;
grabList = grabList->next;
ge->next = lastMatch->next;
lastMatch->next = ge;
}
else
{
/* insert a new grab at end of list */
AddGrab( widget, False, False, keyboard_focus );
if (grabList->next != NULL)
{
ge = grabList;
grabList = grabList->next;
ge->next = NULL;
for (gl = grabList; gl->next != NULL; gl = gl->next);
gl->next = ge;
}
}
return True;
}fred@hpcvlx.HP.COM (Fred Taft) (08/16/88)
### bug number: 402
### area: Xt
### severity: medium
### comments:
VERSION:
Xtk release 2
SYNOPSIS:
XtGetValues() returns bogus information on 68000 architecture
machines, if the application queries a short or char value, and
it also specifies within the arglist a pointer to a short or char
variable, into which the result is to be placed.
DESCRIPTION:
Several months ago, I reported a discrepancy in how XtGetValues() worked,
versus how it was documented. I received a message that the problem was
being fixed, yet I notice that it is still present in the R2 toolkit,
to a degree anyways. According to the documentation, when you issue a
GetValue request, you pass in an arglist composed of an argument
name/address pair; the address indicates where the return value should be
placed. If I don't supply a pointer (i.e. I pass in a NULL address),
when I query a Boolean value, the value is always returned in the arglist;
I noticed that CopyToArg() specifically tested for this case; I can live
with this, since it allows old clients to continue to run. However, if
I do pass in an address, then the returned boolean value was always 0
(FALSE).
After further investigation, I believe the problem lies in the following
line of code which had been added to CopyToArg() to fix the original
problem:
if (*dst != NULL)
dst = *(XtArgVal **)dst;
The above statement treats everything as a pointer to a long variable;
XtArgVal is typedef'ed to a long. In the days where the values were
always returned in the arglist, treating the destination as a long was
fine since the value field really was a long. However, since the
value now pointed to by dst can be a char, a short, a long, etc, this
assumption is no longer valid. For instance, assume the following:
dst ----> arglist value entry ----> a boolean variable
("---->" denotes 'pointing to')
When the buggy statement shown above is executed, the picture now
looks like the following:
dst ----> a boolean variable
Since the argument being queried is a char, the following is used to
copy the value into the specified destinatioin:
*dst = (XtArgVal) *((char *) src);
Since the value pointed to by dst is assumed to be a long, the following
four bytes are copied into the location we specified:
0x00, 0x00, 0x00, 0x01
Unfortunately, at least for those of us using 68000 architecture, the
first 0x00 is copied into our destination, instead of the 0x01.
REPEAT-BY:
Boolean flag;
Arg arg[1];
XtSetArg (arg[0], XtNsensitive, &flag);
XtGetValues (w, arg, XtNumber(arg));
FIX:
/* Patch to Resources.c */
64a65,66
> Boolean addrGiven = False;
>
66a69
> {
67a71,72
> addrGiven = True;
> }
72c77,82
< *dst = (XtArgVal) *((short *) src);
---
> {
> if (addrGiven)
> *((short *)dst) = (short)*((short *) src);
> else
> *dst = (XtArgVal) *((short *) src);
> }
74c84,89
< *dst = (XtArgVal) *((char *) src);
---
> {
> if (addrGiven)
> *((char *)dst) = (char)*((char *) src);
> else
> *dst = (XtArgVal) *((char *) src);
> }casey@admin.cognet.ucla.edu (Casey Leedom) (08/18/88)
Ack!!! Granted that you say your patches are to virgin X11R2 code, it would still be nice if you used context diffs. I don't know how the X Windows Project people are handling their source, but even if it's relatively easy for them to access the virgin 11.2 sources, they're going to have to look twice as hard at your patches just trying to find out where they may actually belong. I really don't blame them for being slow about incorporating your changes. If someone sent me an ed diff for 2.10BSD, I'd just toss it and ask them to resend it as a context diff. Casey