At Fri, 3 Jul 2009 16:04:39 -0400 (EDT), der Mouse
<mouse%Rodents-Montreal.ORG@localhost> wrote:
Subject: Re: why cast to char* through void*
>
> Thor is right from what I might characterize as C's original target: an
> implementation languages for things, such as the low levels of
> operating systems, where type-unsafe operations such as bytewise access
> to structures, or treating pointers as the bit patterns that make them
> up, are fundamentally necessary.
Well, yes, but that's where I'm trying to say it is appropriate to use
/*LINTED*/ comments. These kinds of pointer aliasing tricks are indeed
sometimes necessary, but it is not appropriate to have lint (or the
compiler) always ignore the fact that the programmer is doing something
behind the compiler's back, and just as importantly not ignore the fact
that this may also be doing something a future maintainer will not
immediately understand either. At worst the /*LINTED*/ comment, even
without any additional explanation, is a sign to future maintainers that
they should watch out for something unexpected.
Remember too the related advice of some of the greats in this fine field
of endeavour:
Premature optimization is the root of all evil in programming.
-- C.A.R. Hoare
We should forget about small efficiencies, say about 97% of the
time: premature optimization is the root of all evil.
-- Donald Knuth (re-quoting Tony Hoare)
The key to performance is elegance, not battalions of special cases.
The terrible temptation to tweak should be resisted unless the
payoff is really noticeable.
--Jon Bentley and Doug McIlroy (paraprhasing Tony Hoare)
I think that advice applies particularly in this case of login_cap.c
where it is user-level code that has no significant performance
implications and yet it is currently "abusing" the language's low-level
capabilities, and all essentially just for the sake of showing how smart
the programmer was, even though I think a slightly smarter (or less
hasty) programmer could have avoided both language trickery and the
unnecessary additional storage allocation and copying of the data.
The first time I looked at this code was before Christos made the
initial change to transform some not-so-obvious pointer arithmetic into
a much more clear use of array indexing of what is initially declared to
be an array to find the address of the first unused entry in the array
(i.e. the point where the string can be copied into the same storage
area). Now the more I look at it the more I understand that even the
merging of storage allocation for the pointer array and the string its
pointers point at is unnecessary complexity for an unnoticeable
improvement in performance.
Also, the non-static, yet un-documented and otherwise unused, function
this code occurs in has what on first glance looks to be an overly
complex API for what at first I thought was no valid reason. Here I
refer to the use of a function pointer which is only ever set to one,
static, one-line, function.
Sadly in searching for other uses of setuserenv() I found a
now-incompatible copy of the whole thing with a different name and a
different calling interface, and a now different implementation,
squirrelled away in OpenSSH:
crypto/dist/ssh/session.c:lc_setuserenv(char ***env, u_int *envsize,
login_cap_t *lc)
Someone might want to eliminate this brain-damage since it currently
means use of backslash escapes in the environment settings in
login.conf(5) is not possible with OpenSSH. That probably means
documenting setuserenv() (and perhaps the other public but as-yet
undocumented functions) so that the OpenSSH folks will take back the
changes and do away with their private copy of the code.
--
Greg A. Woods
Planix, Inc.
<woods%planix.com@localhost> +1 416 218-0099 http://www.planix.com/
Attachment:
pgpzN6PSVWwre.pgp
Description: PGP signature