clwords bugfix

clwords bugfix

[Zefram]

-----BEGIN PGP SIGNED MESSAGE-----

There is a bug in zle_tricky.c (ooh, what a surprise).  When the
clwords array needs to be resized, the new part of the array is
supposed to be initialised to NULLs.  The current version has
an off-by-one error, meaning that it clears the current element
(which is immediately freed, so there is only a memory leak here),
but it doesn't clear the new last element (this causes crashes).
The patch below fixes this, and changes it to use a looped assignment
rather than memset, as NULL is not all-bits-zero on all machines.

 -zefram

      Index: Src/zle_tricky.c
      *** zle_tricky.c	1996/05/25 13:14:10	1.22
      --- zle_tricky.c	1996/05/25 13:33:10
      ***************
      *** 998,1006 ****
        	 * more complicated compctl -x things).  They are stored in the     *
        	 * clwords array.  Make this array big enough.                      */
        	if (i + 1 == clwsize) {
        	    clwords = (char **)realloc(clwords,
        				       (clwsize *= 2) * sizeof(char *));
      ! 	    memset((void *) (clwords + i), 0, (clwsize / 2) * sizeof(char *));
        	}
        	zsfree(clwords[i]);
        	/* And store the current token string. */
      --- 998,1008 ----
        	 * more complicated compctl -x things).  They are stored in the     *
        	 * clwords array.  Make this array big enough.                      */
        	if (i + 1 == clwsize) {
      + 	    int n;
        	    clwords = (char **)realloc(clwords,
        				       (clwsize *= 2) * sizeof(char *));
      ! 	    for(n = clwsize; --n > i; )
      ! 		clwords[n] = NULL;
        	}
        	zsfree(clwords[i]);
        	/* And store the current token string. */

-----BEGIN PGP SIGNATURE-----
Version: 2.6.i

iQCVAgUBMacbanD/+HJTpU/hAQFLBwP/Ygx3Hr5KQFL0cGSoi7GAHWMp1yZxgC3B
jILcjEo6Wp+LJPcwA+vqmRrPIUad7SYIWzzi+9h46VcedepDuG/aIYpvEojYBOFr
MDBsKmJD9HBONKStYYfNc6+ruuMS2pw1mW1Kv9fJTtBzMo1zOFpfkQvaS5OtzFtd
u+Zxabwl3Z4=
=k2tq
-----END PGP SIGNATURE-----

Re: clwords bugfix

[Richard Coleman]

> There is a bug in zle_tricky.c (ooh, what a surprise).  When the
> clwords array needs to be resized, the new part of the array is
> supposed to be initialised to NULLs.  The current version has
> an off-by-one error, meaning that it clears the current element
> (which is immediately freed, so there is only a memory leak here),
> but it doesn't clear the new last element (this causes crashes).
> The patch below fixes this, and changes it to use a looped assignment
> rather than memset, as NULL is not all-bits-zero on all machines.
> 
>  -zefram

What Unix has a NULL that isn't all-bits-zero?  That would break
a tremendous amount of code.  There all lots of places in zsh
that assume this and use memset.  I don't think we should change
this.

rc

Re: clwords bugfix

[Zefram]

>What Unix has a NULL that isn't all-bits-zero?

I'm not sure about Unices, but the C FAQ does list more than one system.

>                                                That would break
>a tremendous amount of code.  There all lots of places in zsh
>that assume this and use memset.  I don't think we should change
>this.

I haven't noticed any other places that use memset for this purpose.  I
only noticed this one because I was looking for this particular bug.  I
think if there are other places that do this we should fix them because
they are broken.  GCC can probably optimise looped assignments to be
almost as good as memset on systems where NULL is all-bits-zero.

-zefram

Re: (NULL == 0) ?

[Richard Coleman]

> >What Unix has a NULL that isn't all-bits-zero?
> 
> I'm not sure about Unices, but the C FAQ does list more than one system.

I would be willing to bet money it's a non-unix box.  Just think of
the tremendous amount of unix code that uses the fact that calloc (in
our case zcalloc) returns memory that is all zeros (and hence is
NULL).  Since zsh only works on unix boxes, I think zeroing out memory
with memset is ok.

> >                                                That would break
> >a tremendous amount of code.  There all lots of places in zsh
> >that assume this and use memset.  I don't think we should change
> >this.
> 
> I haven't noticed any other places that use memset for this purpose.  I
> only noticed this one because I was looking for this particular bug.  I
> think if there are other places that do this we should fix them because
> they are broken.  GCC can probably optimise looped assignments to be
> almost as good as memset on systems where NULL is all-bits-zero.

I'm no expert on compilers, but I would be surprised if gcc could
optimize such a loop to be as fast as memset.  Usually memset is
hand optimized assembler code.

The function `newhashtable' uses zcalloc to allocate hashtable memory
and the function `emptyhashtable' uses memset to set this same memory
to zero when it is emptied.  If assuming NULL was equal to 0 would
crash a machine, then these two pieces of code would have done it a
long time ago.  The hashtable code gets executed a lot.

rc

Re: (NULL == 0) ?

[Zefram]

To answer the subject line, (NULL == 0) *is* guaranteed.  0 is the
representation of the null pointer constant in C.

>I would be willing to bet money it's a non-unix box.  Just think of
>the tremendous amount of unix code that uses the fact that calloc (in
>our case zcalloc) returns memory that is all zeros (and hence is
>NULL).  Since zsh only works on unix boxes, I think zeroing out memory
>with memset is ok.

A lot of C code assumes this, but it is broken.  No Unix standard
guarantees more than the C standard about the representation of null
pointers.

>I'm no expert on compilers, but I would be surprised if gcc could
>optimize such a loop to be as fast as memset.  Usually memset is
>hand optimized assembler code.

True, it generally won't be as good, but GCC does produce very
efficient code.  And the speed isn't really critical -- for example, in
this particular case, the code is only executed when you complete on a
command line with 16 words, and it then only needs to assign to 16
pointers.  (It could only cause a crash when you reach 32 words, and on
my system it took 64.)

>The function `newhashtable' uses zcalloc to allocate hashtable memory
>and the function `emptyhashtable' uses memset to set this same memory
>to zero when it is emptied.  If assuming NULL was equal to 0 would
>crash a machine, then these two pieces of code would have done it a
>long time ago.  The hashtable code gets executed a lot.

I can believe that zsh is not currently being used on any system where
the null pointer is not all-bits-zero.  Consequently it's not a really
important issue.  But code that makes this assumption is ill-formed.

-zefram

Re: (NULL == 0) ?

[Zoltan Hidvegi]

> To answer the subject line, (NULL == 0) *is* guaranteed.  0 is the
> representation of the null pointer constant in C.
> 
> >I would be willing to bet money it's a non-unix box.  Just think of
> >the tremendous amount of unix code that uses the fact that calloc (in
> >our case zcalloc) returns memory that is all zeros (and hence is
> >NULL).  Since zsh only works on unix boxes, I think zeroing out memory
> >with memset is ok.
> 
> A lot of C code assumes this, but it is broken.  No Unix standard
> guarantees more than the C standard about the representation of null
> pointers.

Does anything guarantees that on those (theoretical) systems where NULL is
not really zero, all static and external pointers are initialized to this
NULL?  There are many many places where this assumption is used.

Does anyone heard about such a Unix system?  I bet that if such a system
exists memset would be just the easiest among many other problems.

A couple of other questions about Unix and C standards: does anything
guarantee that a variable is always 8 * sizeof(var) bits long?  Does any
Unix or C standard guarantee that ASCII is used (zsh assumes ASCII in many
places)?  Does any Unix or C standard guarantees that an int always has at
least 32 bits?  Did anyone tried to compile zsh on a 80286 of 8086 Unix?
I can imagine that NULL is not really zero on such systems but I doubt
that zsh can be used on such a system anyway.

Zoltan

Re: (NULL == 0) ?

[Bart Schaefer]

On May 27,  2:00am, Zoltan Hidvegi wrote:
} Subject: Re: (NULL == 0) ?
}
} > To answer the subject line, (NULL == 0) *is* guaranteed.  0 is the
} > representation of the null pointer constant in C.

Which means only that an ANSI/ISO C compiler guarantees that

    (pointer == 0)	returns 1 for a null pointer, 0 for non-null
    (pointer != 0)	returns 0 for a null pointer, 1 for non-null
    (pointer = 0)	assigns the null bit pattern

} > A lot of C code assumes [null bit pattern is 0], but it is broken.
} 
} Does anything guarantees that on those (theoretical) systems where NULL is
} not really zero, all static and external pointers are initialized to this
} NULL?

Yes, an ANSI/ISO C compiler must do this.

} Does anyone heard about such a Unix system?

This isn't a UNIX-or-not issue, it's a hardware issue.  I don't offhand
know of any such hardware that runs any well-known UNIX, but there are
probably some UNIX-like or UNIX-emulation operating systems for some
such hardware.

} I bet that if such a system
} exists memset would be just the easiest among many other problems.

True.  Very likely the C compilers on those systems at least have a
switch to cause them to reserve the address 0 and to treat pointers
that reference that address as NULL.  It would require extra code at
every pointer comparison, so it wouldn't be very efficient, but it
wouldn't be impossible.  I've never had direct experience with this,
though.

} A couple of other questions about Unix and C standards: does anything
} guarantee that a variable is always 8 * sizeof(var) bits long?

Not exactly.  ANSI/ISO C says something like this:

The value representable by (char)  is always AT LEAST  8 bits
                           (short)                    16
                           (long)                     32
and sizeof(char) is always 1, and sizeof(anything) is integral, and
sizeof(long) >= sizeof(int) >= sizeof(short) >= sizeof(char); and that
values returned by malloc/calloc/etc. point to memory correctly byte-
aligned for any type.

I don't recall whether sizeof(int) must be the same as one of (short)
or (long).  I don't think so (don't have an ANSI C book handy at the
moment).

But nothing prohibits, say, 11-bit chars with 33-bit longs.

} Does any Unix or C standard guarantee that ASCII is used (zsh assumes
} ASCII in many places)?

ANSI/ISO C guarantees that character constants are interpreted as the
corresponding ASCII numeric values when assigned or compared to integral
types, if that's what you mean.

} Does any Unix or C standard guarantees that an int always has at
} least 32 bits?

Definitely not.  See above.

Zsh does work when sizeof(int) != sizeof(long), e.g 4 != 8, because
I've run it on a DEC Alpha.  Well, older versions worked, anyway.

} Did anyone tried to compile zsh on a 80286 or 8086 Unix?

There's Linux for the 8086, you know ... terrifying.

Anybody out there running SCO UNIX care to compile in Xenix-compatible
286-mode and watch the fun?

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: (NULL == 0) ?

[anthony baxter]

> I don't recall whether sizeof(int) must be the same as one of (short)
> or (long).  I don't think so (don't have an ANSI C book handy at the
> moment).

No, they dont (and on the Alpha, for example, they aren't - short=2bytes,
int=4bytes, long=8bytes)

> Zsh does work when sizeof(int) != sizeof(long), e.g 4 != 8, because
> I've run it on a DEC Alpha.  Well, older versions worked, anyway.

beta 18 works. haven't got beta19. I seem to remember people posting
that some of the betas at least it also works on Unicos, where
sizeof(int)==sizeof(long)==8.

Anthony

Re: (NULL == 0) ?

[Zefram]

>Does anything guarantees that on those (theoretical) systems where NULL is
>not really zero, all static and external pointers are initialized to this
>NULL?  There are many many places where this assumption is used.

Yes.  All variables with static duration are initialised as if they had
been explicitly initialised with `= 0', so integers get 0, floating
point variables get a floating point 0.0, and pointers get NULL.

>Does anyone heard about such a Unix system?  I bet that if such a system
>exists memset would be just the easiest among many other problems.

The only significant problems I am aware of are use of memset/calloc,
and assumptions about the result of treating a pointer as an integer.
I'm pretty sure zsh never casts a pointer to an integral type, so
memset/calloc is the only thing we need to worry about.

>A couple of other questions about Unix and C standards: does anything
>guarantee that a variable is always 8 * sizeof(var) bits long?

Only if a byte is 8 bits long and there are no holes in the integral
types.  This type of holes are very rare, and we assume 8 bit bytes
elsewhere.  I don't think we actually assume this directly anyway.

>                                                                Does any
>Unix or C standard guarantee that ASCII is used (zsh assumes ASCII in many
>places)?

No.  That is certainly an issue, but it's going to be very difficult to
resolve.

>          Does any Unix or C standard guarantees that an int always has at
>least 32 bits?

No.  But C guarantees that long is at least 32 bits, so if there's
anywhere we need 32 bits we should change it to use long.  I don't know
of anywhere in zsh tht needs fixing in this respect.

The C standard guarantees the following:

CHAR_BIT >= 8

SCHAR_MAX >=  0x7f
SCHAR_MIN <= -0x7f
UCHAR_MAX >=  0xff

There are no holes in char.

SHRT_MAX  >=  0x7fff
SHRT_MIN  <= -0x7fff
USHRT_MAX >=  0xffff

I.e. short is at least 16 bits.

INT_MAX   >=  0x7fff
INT_MIN   <= -0x7fff
UINT_MAX  >=  0xffff

I.e. int is at least 16 bits.

LONG_MAX  >=  0x7fffffff
LONG_MIN  <= -0x7fffffff
ULONG_MAX >=  0xffffffff

I.e. long is at least 32 bits.

It is further guaranteed that SCHAR_MAX SHRT_MAX INT_MAX LONG_MAX is a
nondecreasing sequence, as is UCHAR_MAX USHRT_MAX UINT_MAX ULONG_MAX,
and SCHAR_MIN SHRT_MIN INT_MIN LONG_MIN is a nonincreasing sequence.
The definition of the standard library indirectly requires SHRT_MAX >=
UCHAR_MAX.

The maximum values of the unsigned types are required to be of the form
2^n-1, where n is an integer.  There are also some requirements
relating each unsigned type to its signed counterpart -- basically
Ufoo_MAX > foo_MAX.

sizeof(char) sizeof(short) sizeof(int) sizeof(long) is also required to
be a nondecreasing sequence, and for each integral type sizeof(unsigned
foo) == sizeof(signed foo).

-zefram

Re: (NULL == 0) ?

[Zoltan Hidvegi]

> >A couple of other questions about Unix and C standards: does anything
> >guarantee that a variable is always 8 * sizeof(var) bits long?
> 
> Only if a byte is 8 bits long and there are no holes in the integral
> types.  This type of holes are very rare, and we assume 8 bit bytes
> elsewhere.  I don't think we actually assume this directly anyway.

It is used to determine the size of buffers to store the string
representation of an integer.  Just grep for 'SIZEOF_LONG \* 8'.

> >          Does any Unix or C standard guarantees that an int always has at
> >least 32 bits?
> 
> No.  But C guarantees that long is at least 32 bits, so if there's
> anywhere we need 32 bits we should change it to use long.  I don't know
> of anywhere in zsh tht needs fixing in this respect.

ntype in struct node has to be at least 32 bit long but it is defined to
int.  But it would be wasting of memory to use long on systems where int
has 32 bits and long has 64.

In hashtable.h it is assumed that sizeof(void*) >= sizeof(long).

Zoltan

Re: (NULL == 0) ?

[Zefram]

>It is used to determine the size of buffers to store the string
>representation of an integer.  Just grep for 'SIZEOF_LONG \* 8'.

If we change the `8' to `CHAR_BIT' (which will need to be determined at
configuration time if there isn't a limits.h), it would be more
portable.  But we are assuming 8-bit bytes, as well as ASCII, in a lot
of character handling code.

>ntype in struct node has to be at least 32 bit long but it is defined to
>int.  But it would be wasting of memory to use long on systems where int
>has 32 bits and long has 64.

It might be wise to do a configuration check for the smallest type of
at least 32 bits.  There are a number of bitfields that could use
this.

>In hashtable.h it is assumed that sizeof(void*) >= sizeof(long).

We should have a union there, as we do elsewhere.  long and void * are
sufficient, but it might be neater to have some other pointer types too
(it would reduce casting, and would not decrease efficiency).

-zefram

Re: (NULL == 0) ?

[Zoltan Hidvegi]

> >It is used to determine the size of buffers to store the string
> >representation of an integer.  Just grep for 'SIZEOF_LONG \* 8'.
> 
> If we change the `8' to `CHAR_BIT' (which will need to be determined at
> configuration time if there isn't a limits.h), it would be more
> portable.  But we are assuming 8-bit bytes, as well as ASCII, in a lot
> of character handling code.

Bart wrote that ANSI C guarantees ASCII in the cases we need so that should
not cause any problems.  I do not see where is is assumed that a char has 8
bits othar than buffer sizes.  We do assume that char has at least 8 bits
but that's OK.

> >In hashtable.h it is assumed that sizeof(void*) >= sizeof(long).
> 
> We should have a union there, as we do elsewhere.  long and void * are
> sufficient, but it might be neater to have some other pointer types too
> (it would reduce casting, and would not decrease efficiency).

Here struct iparam is used for initialisation.  It is used instead of
struct param since initialisation of a union might cause problems.

Zoltan

Re: (NULL == 0) ?

[Zefram]

>Bart wrote that ANSI C guarantees ASCII in the cases we need so that should
>not cause any problems.

C doesn't guarantee *any* use of ASCII.  It does make some guarantees
about the character set, but nothing involving actual ASCII.
(Basically, all the characters that C uses, plus the characters with
escapes like '\a', must be in the character set and distinct, and none
may have the value 0.)

>                         I do not see where is is assumed that a char has 8
>bits othar than buffer sizes.  We do assume that char has at least 8 bits
>but that's OK.

ZLE has some 256 element tables indexed by unsigned chars that are read
from the tty.  If char has more than 8 bits, the tables need to be made
bigger.  There are a couple of `& 0xff's, but they're easily fixed.


>Here struct iparam is used for initialisation.  It is used instead of
>struct param since initialisation of a union might cause problems.

There's definitely a problem here.  sizeof(long) > sizeof(void *) isn't
an unreasonable situation.  In fact, it makes a great deal of sense to
have 64-bit longs and 32-bit pointers on Sparc processors.  On Sparc
v9, the natural size of int and void * is 64 bits (though 32-bit
pointers are still possible), and 128-bit longs would be sensible.  As
a union isn't usable here, should we perhaps have two distinct iparam
types, one for integers and one for pointers?

-zefram

Re: (NULL == 0) ?

[Bart Schaefer]

On May 28, 12:00am, Zoltan Hidvegi wrote:
} Subject: Re: (NULL == 0) ?
}
} > >A couple of other questions about Unix and C standards: does anything
} > >guarantee that a variable is always 8 * sizeof(var) bits long?
} > 
} > Only if a byte is 8 bits long and there are no holes in the integral
} > types.  This type of holes are very rare, and we assume 8 bit bytes
} > elsewhere.  I don't think we actually assume this directly anyway.
} 
} It is used to determine the size of buffers to store the string
} representation of an integer.  Just grep for 'SIZEOF_LONG \* 8'.

Hmm, shouldn't params.c be using the DIGBUFSIZ constant from system.h
everywhere, rather than [(SIZEOF_LONG * 8) + 4] in spots?

} ntype in struct node has to be at least 32 bit long but it is defined to
} int.  But it would be wasting of memory to use long on systems where int
} has 32 bits and long has 64.

I don't think that amount of extra memory usage should be of very much
concern.  How many 64-bit machines with small amounts of RAM or swap
would you really expect to encounter?  What's the point of having all
that address space if you can't use it?

On May 27, 11:11pm, Zefram wrote:
} Subject: Re: (NULL == 0) ?
}
} It might be wise to do a configuration check for the smallest type of
} at least 32 bits.  There are a number of bitfields that could use
} this.

Sigh.  I really hate having to clutter up code with "Int32" typedefs
and their ilk.  The only place this should really be necessary is in
networking code or the like, where you have to use exactly the number
of bits that your peer is sending.  (Try running an X11 client from
a DEC Alpha and displaying on an X11R4 server on some 32-bit machine.
Gets ugly fast.  Even worse if the server is a Windows 3.1 X server
built with 16-bit MSC.)

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"

Re: (NULL == 0) ?

[Bart Schaefer]

On May 27, 11:54pm, Zefram wrote:
} Subject: Re: (NULL == 0) ?
}
} >Bart wrote that ANSI C guarantees ASCII in the cases we need so that should
} >not cause any problems.
} 
} C doesn't guarantee *any* use of ASCII.  It does make some guarantees
} about the character set, but nothing involving actual ASCII.

Oops, you're right.  ANSI only guarantees that character constants will
behave "as if" they were read by the stdio library.  Sorry about that;
I should have remembered, for example, that perfectly good C compilers
on the Mac swap the integer values of '\r' and '\n' so that Mac text
files will appear to have '\n'-terminated lines.

Does zsh assume ASCII ordering of e.g. 'a' .. 'z' anywhere?  Or only
that all of 'a' .. 'z' etc. are in the range 1 .. 255?

I tend to agree with RC that trying too hard to deal with all these
obscure cases will only cause more problems than it solves.  If we
have to do something, perhaps a simple configuration test for ASCII
character ordering of the alphanumerics would be sufficient.

} (Basically, all the characters that C uses, plus the characters with
} escapes like '\a', must be in the character set and distinct [...])

It doesn't even guarantee that, actually, now that my memory has been
jogged.  Trigraphs were invented because some characters like '[' and
']' aren't in some character sets (e.g. ISO 646).

-- 
Bart Schaefer                             Brass Lantern Enterprises
http://www.well.com/user/barts            http://www.nbn.com/people/lantern

New male in /home/schaefer:
>N  2 Justin William Schaefer  Sat May 11 03:43  53/4040  "Happy Birthday"