[comp.windows.x] Fixes to paint and xedit in V11.2

jkh@violet.berkeley.edu (Jordan K. Hubbard) (07/24/87)

Ed Moy and I found a few irritating bugs..

1. in line 232 of clients/xedit/xedit.c, rindex is blindly called
	with argv[1] to see if it's a display argument. On the
	vax, null pointers can be deref'd, so no problem.. On the
	Sun, however.. This is fixed by checking to see if the arg
	is null first..

2. in line 150 of clients/paint/main.c, XDrawImageString is called
	without a display argument! You hosers!


I said it once before last year and I'll say it again:

	"Someday MIT will discover lint.."

					Jordan Hubbard
					Ed Moy
					U.C. Berkeley

mlandau@bbn.com (Matt Landau) (07/25/87)

In comp.windows.x jkh@violet.berkeley.edu (Jordan K. Hubbard) writes:
>Ed Moy and I found a few irritating bugs..
>
>1. in line 232 of clients/xedit/xedit.c, rindex is blindly called
>	with argv[1] to see if it's a display argument. On the
>	vax, null pointers can be deref'd, so no problem.. On the
>	Sun, however.. This is fixed by checking to see if the arg
>	is null first..

It's worse than that.  The entire Xt Toolkit blithely assumes that 
you can pass arbitrary pointers to strcmp, strcpy, index, etc.  Any
program that uses the toolkit and tries to do, for instance, buttons,
will crash on a 68k.  (Yes, I *KNOW* the toolkit is considered in
alpha release, but seriously, this strikes me as the kind of thing 
that shouldn't creep into the code in first place.)

I fixed the toolkit by replacing all the calls to the various string
functions with versions called XtStrCmp, XtStrCpy, etc. that handle
NULL pointer arguments as would a VAX.  I'll be sending out a bug
report on the X11 toolkit that contains diffs for this.

HOWEVER, as Jordan and Ed discovered, just fixing the toolkit doesn't
do it, since some of the clients are also broken in how they deal 
with pointers.  Rather than try to find and fix all the bogosity in
the clients, I punted and installed my own (VAX-style) versions of
all the string routines in libX11.a -- if you can't make 'em fix it,
at least you can make it work.
-- 
 Matt Landau			Oblivion gallops closer,
 mlandau@bbn.com		    favoring the spur, sparing the rein...

guy%gorodish@Sun.COM (Guy Harris) (07/27/87)

> I fixed the toolkit by replacing all the calls to the various string
> functions with versions called XtStrCmp, XtStrCpy, etc. that handle
> NULL pointer arguments as would a VAX.

More correctly, as certain C and operating-system implementations do
on a VAX.  Unless the VMS versions of those routines waste time
checking for NULL arguments beforehand, they will blow up too, since
by default VMS does not map location 0 into a process' address space.

Furthermore, there are VAX UNIX implementations that permit you to
take location 0 out of a process' address space; the paging System V
implementations do, and John Bruner has made changes to 4BSD to
permit it to as well.  If somebody wants to link their programs on
such a system with the "disallow null pointer references" option,
they're going to be screwed unless the toolkit is fixed.  Not only
that, but the Berkeley people may very well decide to make no-page-0
the default if they rewrite the VM system in some future release.

If the people doing X development intend to continue doing it on
VAXes running 4BSD, they are hereby advised to try to get a hold of
John Bruner's changes, put them into their OS, and link *all* their
programs with the "no page 0" option, and catch more of these
problems before the software goes out the door.
	Guy Harris
	{ihnp4, decvax, seismo, decwrl, ...}!sun!guy
	s tr329idssh

haynes@DECWRL.DEC.COM (07/28/87)

Being one of the primary designers and implementors of the "Xt"
toolkit, I'd like to respond to the recent rash of criticisms of null
pointer handling in the toolkit.

First, please do recognize the distinction between the applications
(clients) and the toolkit itself. The bugs that Jordan and Ed reported
were both in clients (xedit and paint). I can't control what other
people do with our code.

Second, I don't know of any places in the toolkit that allow passing
NULL to the string routines. If any such exist they are bugs. PLEASE
REPORT THEM!!! We can't fix bugs that we never see! I have see NO that
is NOT ONE bug report from the people complaining about null handling
in the toolkit. You want to gripe, or you want to be helpful? Send your
bug reports to Todd at athena, and they'll get sent to the right people.

By the way, passing a null string as a command label is illegal.
Programs that do this should die (and will on a 68k). You can argue
whether or not this is good, but that's the intended behavior. So the
fact that we blithely pass such strings to strcmp etc. is not evidence
of our VAX/Unix chauvinism, it's evidence of our laxness in
documentation. Which flaw I freely admit and unashamedly ask for help
with! Any of you kind folks out there willing to DONATE time to help us
write toolkit documentation? After all, I'm personally making big bucks
sitting here answering mail about the toolkit...

	"If you can't make 'em fix it..."

grrr...

	-- Charles