On Thursday 18 September 2008 12:03:19 NotFound wrote: > On Thu, Sep 18, 2008 at 8:14 PM, chromatic <[EMAIL PROTECTED]> wrote:
> > The comment helps, but defining these magic numbers as magic constants > > might be even clearer (but please keep the comment). > I think that setting a constant is sort of establishing it as a > documented value, and I don't think that document who many digits can > have the hex representation of flags is good, or it must be done in a > file other than the one that defines the flags. Even #define-ing them locally to the function has readability value. I want the code to be so blindingly obvious to read that if there's a problem, it's apparent without having to turn magic numbers back into their meanings. The compiler doesn't care either way. > >> + PARROT_ASSERT(strlen(buf) + strlen(s) < bufsize); > >> + strcat(buf, s); > > > > That only helps with debug builds; if there are possible inputs we won't > > discover in our testing, we might as well make this an unconditional test > > and throw an exception if something goes wrong here. IMCC is > > user-facing, so I want to be paranoid. > > After some thinking I must agree. I hope that this code will be > refactored before 1.0, but we can't be sure. I sort of hope IMCC can vanish before 1.0, or at least find a small army of heavily motivated cage cleaners who merge it with pirc. > The problem is that I don't know what severity must be used, and if it > the exception must be catched by the main imcc entry points or let it > propagate to the caller. Maybe all imcc fail conditions must throw > exceptions, for the benefit of load_bytecode invocations. I don't think anyone's asked that question yet (at least no one has answered it yet), so let's not let that get in the way of adding an exception here now. Unifying IMCC exceptions with Parrot exceptions does seem valuable (though convincing IMCC not to leak memory under those circumstances... I'm going to lie down now for a while). -- c
