[Poppler-bugs] [Bug 89076] [patch] fix "length bigger than vheaTab size" and "length bigger than vmtxTab size"

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=89076

--- Comment #2 from Adrian Johnson <ajohnson at redneon.com> ---
These three changes are unrelated and should be split into three separate
commits.

My only concern with the alloca change is ensuring the stack usage is not too
large.

>  argsSize = 8;
> +#if HAVE_ALLOCA_H
> +  args = (GooStringFormatArg *)alloca(argsSize * sizeof(GooStringFormatArg));
> +#else

This is a fixed size allocation that I estimate to be up to 544 bytes so it
should be fine.

>  	    argsSize *= 2;
> +#if HAVE_ALLOCA_H
> +	    {
> +	      GooStringFormatArg *new_args = (GooStringFormatArg
>      *)alloca(argsSize * sizeof(GooStringFormatArg));
> +	      memcpy(new_args, args, argsLen);
> +	      args = new_args;
> +	    }
> +#else

It looks like the allocation is increased if there are more than 8 format args.
I have not searched the code to see if there are more than 8 format args used.
I'm guessing this is unlikely so maybe when there are more than 8 args we could
fallback to using gmallocn/greallocn. If > 8 args is rare there will be little
performance impact but it does put an upper bound on the stack usage.

> +#if HAVE_ALLOCA_H
> +#else
>   gfree(args);
> +#endif

#ifndef HAVE_ALLOCA_H
   gfree(args);
#endif

would be better.

> +#if 0
> int FlateStream::getChar() {
>   if (pred) {
>     return pred->getChar();
>   }
>   return doGetRawChar();
> }
> +#endif

I don't see any need for keeping this. It will always be in the git history.


I had trouble understanding what the timings you provided are referring to.
Could you provide separate times for master, master + alloca change, and master
+ alloc + inline change.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/poppler-bugs/attachments/20150211/a2fd344a/attachment.html>