On Mon, Jul 30, 2007 at 09:20:27PM -0700, Matt Diephouse wrote:
> On 7/30/07, chromatic <[EMAIL PROTECTED]> wrote:
> > On Monday 30 July 2007 00:21:09 [EMAIL PROTECTED] wrote:
> > > Author: mdiep

> > >=== --- trunk/src/inter_run.c  (original)
> > > +++ trunk/src/inter_run.c     Mon Jul 30 00:21:07 2007
> > > @@ -167,9 +167,7 @@
> > >  {
> > >      opcode_t offset, *dest;
> > >      parrot_context_t *ctx;
> > > -    /*
> > > -     * FIXME argument count limited - check strlen of sig
> > > -     */
> > > +
> > >      char new_sig[10];
> > >      const char *sig_p;
> > >      parrot_context_t * const old_ctx = CONTEXT(interp->ctx);
> >
> > I think this comment meant "Hey, allocating a ten-character array on the 
> > stack
> > might put us in danger of overruns."
> 
> I removed it because down later in the source, the strlen of sig *is* checked:
> 
>         const size_t len = strlen(sig);
>         if (len > 8) {
>             real_exception(interp, NULL, 1, "too many arguments in
> runops_args");
>         }
> 
> The string is only copied after this check is made.

So shouldn't that 8 be sizeof(new_sig) - 1 ?

Right now there are two magic numbers, one of which is actually off by one,
and no clear linking of the two.

Nicholas Clark

Reply via email to