On Mon, 5 Apr 2010, Brad Jorsch wrote:
> > unless i should really be sleeping, the return is either bogus, or
> > 158-165 should quite be killed, no?
>
> Go get some sleep ;) There are three possibilities in this bit of code:
you do have a point ;) i just did :)
> 1. w is panel->icoaB, in which case lPtr is panel->icoL
> 2. w is panel->pixaB, in which case lPtr is panel->pixL
> 3. w is something else, in which case lPtr is uninitialized garbage
> Without the return, the code would happily pass the garbage pointer and
> quite possibly wind up with a segfault. With the return, it just does
> nothing. It may be that something other than "do nothing" is the correct
> behavior, but since it looks like case #3 is a "can never happen" case
> it really doesn't matter too much. Maybe change the return to an
> explicit "scream and die" then?
that would be much better, imho, at about the first convenient place
in the function, not buried such deeply, so that it is _obvious_.
in this particular case an assert looks ok to me, but in general, i
find the current way of using asserts quite strange. look at
proplist.c:hashPropList() as an example: even though it is an obvious
code error to call this function with anything but plstring or pldata
arguments, for some reason if you are not running DEBUG, it is quite
forgiving, and buries this fact badly. other places, it likes to throw
up on badly formatted plist input (which is somewhat worse, as
hashPropList is called by someone supposedly having an idea what he is
doing, and a bad proplist might come from a user having made a typo in
a conf file -- so it is all backwards imho). i would be all for doing
away with WINGs/WINGs/WUtil.h:35-67 for something not so convoluted
(like, uh, an assert() :)
as for WPrefs, what would be even better though if you did some
renaming of stuff so they reflect what they _do_ ("createPanel",
indeed it creates a panel, but not just any panel, so why give it a
generic name?).
--
[-]
mkdir /nonexistent
--
To unsubscribe, send mail to [email protected].
