Hrvoje Niksic <hniksic(a)srce.hr> writes:
> XSTRING_DATA is exactly what the name says, dereferencing Lisp_Object
> to struct Lisp_String, and then dereferencing it to a Bufbyte pointer.
> It adhering to all the normal XEmacs naming conventions. XSTRING_DATA
> (logically) returns a Bufbyte pointer, which should cries for
> conversion when used on the outside.
However it seems to be common practise to just cast it to a (char *).
Grepping for char \*)[ ]*XSTRING_DATA still has 223 hits most them
probably very correct code.
However may be I am going mad but I do not get the feeling that almost
all are correct, for instance [randomly taken from igrepping for the
above expression].
glyphs.c:1798: result = XpmReadFileToData ((char *) XSTRING_DATA (name), &data);
I haven't checked on it further but that one is bound to be wrong.
These are also suspect
frame-x.c:1350: strcpy (dnd_data + dnd_len - 1, (CONST char *)XSTRING_DATA (XCAR (run)));
frame-x.c:1365: dnd_data = (char *)XSTRING_DATA (data);
These ones are surely wrong:
fileio.c:1731: else if (stat ((CONST char *) XSTRING_DATA (newname), &out_st) < 0)
fileio.c:1734: ifd = interruptible_open ((char *) XSTRING_DATA (filename), O_RDONLY | OPEN_BINARY, 0);
fileio.c:1773: ofd = open( (char *) XSTRING_DATA (newname),
The one in Fopen_database is clearly wrong also. Also the database
keys are in XEmacs internal incoding. Was that intended?
Anyway that was just a random sample of things I checked. May be it
should be reassuring that I can spot most of these from the grep
output but somehow it isn't.
> People who hack XEmacs should know what they are doing.
However there is still a lot of old pre-MULE code lying around. MULE
changed the meaning of XSTRING_DATA and there is no way of telling
somebody looked at the code to make sure it was correct. Especially
given the fact that since our internal coding is a superset of ascii
it will work OK most of the time.
> If they
> don't, renaming XSTRING_DATA to something else won't help one bit.
What I mean is that renaming XSTRING_DATA forces one to look at every
use of it once. i.e. any use of XSTRING_BUFBYTE or any other new name
makes it immediately clear that particular piece of code was looked at
and found to be correct.
I.e. you replace subtle breakage (XSTRING_DATA no longer ascii) by
explicit breakage (undefined macro) forcing one to look at the code.
This technique is used by Linus in his kernel and I think it works
well (and he does get a lot of flak for it sometimes).
> > I am not advocating doing this now but I have the feeling this
> > should be done sometime and that why this particular number of
> > XSTRING_DATA's look scary.
>
> Why does it look scary? Yes, you spotted an actual bug, and yes, the
> patch for it is trivial.
It looks scary because I have the feeling there are a lot more of
these to be found. It looks scary because a lot these problems are in
code that was supposed to be looked over already (somehow I think
file-system-coding came after Ben's checks). I also think most these
problems in new code comes from people cut&pasting old buggy code
(i.e. process-nt vs process-unix).
> But what's the problem with the normal, non-buggy XSTRING_DATA's in
> the code?
They are indistinguishable from the buggy ones.
As to comments about the "Coding for MULE section" I think there must
be something about when converting TO external strings. You won't
crash XEmacs with it but you will create subtle problems.
Jan
P.S. On a related matter. In buffer.h
You may wonder why this is written in this fashion and not as a
function call. With a little trickery it could certainly be
written this way, but it won't work because of those DAMN GCC WANKERS
who couldn't be bothered to handle alloca() properly on the x86
architecture. (If you put a call to alloca() in the argument to
a function call, the stack space gets allocated right in the
middle of the arguments to the function call and you are unbelievably
hosed.) */
Is this still the case? For instance m/intel386.h defines
#ifdef __GNUC__
/* GCC's alloca() is semi-broken. See lisp.h.
This brokenness has been confirmed under both Linux and NetBSD.
It may also exist on non-Intel architectures. */
#define BROKEN_ALLOCA_IN_FUNCTION_CALLS
#endif
however no nuch define is used anywhere in the code.
P.P.S. FSF Emacs makes of the lisp allocator to allocate the strings.
Therefore they do not need alloca.
However it seems to create a new buffer for each filename it needs to
convert!
Jan
