Jerry James writes:
I had to compile another kernel. :-)
Stop bragging! :-)
This patch [...] may uglify the code a bit (although I don't
really
think that it does; in fact, I generally detest the macro soup I
have to swim in when I go through our code).
I wouldn't go so far as say I detest it. Having accessors is kind of
Lisp-y, although I think that the prevalence of allcaps identifiers is
ugly, and it's certainly a barrier to entry to new programmers.
I wonder, given your statement that we always get error-checking in an
error-checking build anyway, if it might be a good idea to take the
next step, and do away with the maybe-error-checking macros entirely,
and use an explicit check as we do in Lisp primitives.
Especially since that doesn't actually seem to be true, eg in this
hunk:
diff -r c064d7197712 src/faces.c
--- a/src/faces.c Sat Apr 18 03:24:48 2009 +0900
+++ b/src/faces.c Fri May 15 14:20:28 2009 -0600
@@ -1544,8 +1544,8 @@
}
cachel->display_table = Qunbound;
cachel->background_pixmap = Qunbound;
- FACE_CACHEL_FONT_SPECIFIED (cachel)->size = sizeof(cachel->font_specified);
- FACE_CACHEL_FONT_UPDATED (cachel)->size = sizeof(cachel->font_updated);
+ cachel->font_specified.size = sizeof(cachel->font_specified);
+ cachel->font_updated.size = sizeof(cachel->font_updated);
}
I assume that the maybe-error-checking isn't called elsewhere, so
where's the error-checking now?
So I suggest the following form:
diff -r c064d7197712 src/faces.c
--- a/src/faces.c Sat Apr 18 03:24:48 2009 +0900
+++ b/src/faces.c Fri May 15 14:20:28 2009 -0600
@@ -1544,8 +1544,10 @@
}
cachel->display_table = Qunbound;
cachel->background_pixmap = Qunbound;
- FACE_CACHEL_FONT_SPECIFIED (cachel)->size = sizeof(cachel->font_specified);
- FACE_CACHEL_FONT_UPDATED (cachel)->size = sizeof(cachel->font_updated);
+ CHECK_FACE_CACHEL_FONT_SPECIFIED (cachel);
+ CHECK_FACE_CACHEL_FONT_UPDATED (cachel);
+ cachel->font_specified.size = sizeof(cachel->font_specified);
+ cachel->font_updated.size = sizeof(cachel->font_updated);
}
I'm not sure that's the right name, maybe the checks should be
something like
+ CHECK_FACE_CACHEL_FONT (cachel->font_specified);
+ CHECK_FACE_CACHEL_FONT (cachel->font_updated);
or even just
+ CHECK_FACE_CACHEL (cachel);
and there are yet other possibilities depending on just what checks
are done, and how they tend to associate.
_______________________________________________
XEmacs-Beta mailing list
XEmacs-Beta(a)xemacs.org
http://calypso.tux.org/cgi-bin/mailman/listinfo/xemacs-beta