On Thu, Sep 18, 2008 at 8:14 PM, chromatic <[EMAIL PROTECTED]> wrote:
>> - char * buf = mem_allocate_n_typed(5*n+1, char *);
>> + /* Assumptions:
>> + * Flags has no more than 3 hex digits
>> + * Plus 0x and , gives 6 char for arg
>> + * 4 more for: "( , )", and
>> + * 1 more for C string 0 delimiter
>> + * Last item has no , but we can forget that to avoid
>> + * to have to check for 0 args.
>> + */
>> + unsigned int bufsize = 6 * n + 5;
>> + char * buf = mem_allocate_n_typed(bufsize, char);
>>
>> strcpy(buf, "\"(");
>> for (i = 0; i < n; i++) {
>
> 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.
>> + 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.
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.
--
Salu2
