A Diumenge 14 Octubre 2007, Jeff Muizelaar va escriure: > On Sun, Oct 14, 2007 at 01:17:05AM +0200, Albert Astals Cid wrote: > > Hi, this patch has two parts. > > > > First, it removes all exit(1) calls in favor or the best i could find, > > that is, return NULL (and hope the caller can understand NULL as error) > > in the gmem routines and the correct ignore in DCTStream. > > What's the motivation for this? Won't the callers just segfault > afterwards anyways? It looks like most callers assume that gammloc > succeeds. If we wanted to deal with the out of memory situation it would > require quite a bit of work to add all of the proper error handling > code. At least with exisiting implementation the application dies > cleanly instead of segfaulting. It would be nice to deal with > out-of-memory properly, but I think it would be quite a bit of work...
The motivation is that without it my splash patch is worthless because the huge font tries to create a huge cache (before i can discard it because it's out of the clip area) that integer wraps a gmallocn call and i got an app exit(), making it return NULL gets it to work without any problem. I agree in most other situations will probably segfault, but as i said exit() in library code is a bad idea, so when we hit those segfaults because of the return NULL we try to fix them instead of the exit(1). Of course i can catch the integer wrapping in the code that calls gmallocn but why duplicating checks everywhere if you can have them at one place only? Albert > > > Second, in Splash code it moves the resposability for checking if a glyph > > is inside the clip rect from Splash::fillGlyph2 to its callers, > > Splash::fillGlyph and Splash::fillChar, that way i can check inside > > SplashFTFont::makeGlyph if the font is inside the clip rect before > > rendering. > > > > This fixes crash due to memory exhaustion on KDE bug 150693 [1] because > > that pdf specifies a HUGE font outside the clip rect :-/ > > Didn't look at the splash stuff. > > > CairoOutputDev does not need to do that because cairo already does it. > > > > If noone objects to the patch (that is a bit intrusive but clean enough > > imho) i will commit it to HEAD and 0.6 branch next friday. > > Regardless of whether this patch goes in or not, make sure you do it in > two commits because the changes are completely unrelated. > > -Jeff > _______________________________________________ > poppler mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/poppler _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
