El Dimarts, 20 de novembre de 2012, a les 08:05:56, Adam Reichold va escriure: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > Hello Albert, > > I am beginning to understand why one would not touch this, if it isn't > necessary. :-( > > I redid the changes and found that I missed to copy > "SampledFunction::nSamples". Sorry. Tested it against the PDF file you > linked.
Now crashes in http://www.it.piaggio.com/media/mp3hybdepita.pdf Cheers, Albert > > On a different but related topic: I tested the first patch using the > test data repository and "make check", but the number of documents in > this set seems limited. But be more helpful (I am also thinking of > Thomas' threading patches), is they a way to get or automatically > generate the collection of PDF files covered by the bug tracker? (Or > whatever else you use for full regression testing.) > > Best regards, Adam. > > Am 19.11.2012 22:44, schrieb Albert Astals Cid: > > El Dilluns, 19 de novembre de 2012, a les 08:16:59, Adam Reichold > > va escriure: Hello Albert, > > > > I am sorry about that. It seems like my editor messed up with the > > spaces since it seems to apply using "--ignore-space-change". I > > tried to reformat the patch using said option, hopefully this > > helps. (It still has some whitespace warnings but no more errors.) > > > >> Can you have a look at > >> https://bugs.freedesktop.org/attachment.cgi?id=18330 > >> > >> The dash line that divides the the green rectangular area changes > >> from grayish to black when applying your patch. > >> > >> Cheers, Albert > > > > Regards, Adam. > > > > On 18.11.2012 23:01, Albert Astals Cid wrote: > >>>> El Dimecres, 14 de novembre de 2012, a les 18:48:26, Adam > >>>> Reichold va escriure: Hello again, > >>>> > >>>> The attached patch should rework the function copy > >>>> constructors. > >>>> > >>>>> Doesn't seem to apply on current master, can you please > >>>>> rebase it and make sure it applies? > >>>>> > >>>>> Cheers, Albert > >>>> > >>>> Best regards, Adam. > >>>> > >>>> On 14.11.2012 12:58, Albert Astals Cid wrote: > >>>>>>> El Dimecres, 14 de novembre de 2012, a les 07:28:21, > >>>>>>> Adam Reichold va escriure: Hello Albert, > >>>>>>> > >>>>>>> Before I try the other classes, could you have a look > >>>>>>> at the > >>>>>>> > >>>>>>> attached patch for SampledFunction and tell if this is > >>>>>>> what > >>>>>>> > >>>>>>> you are looking for? > >>>>>>> > >>>>>>> Would something like memcpy(sampleSize, > >>>>>>> func->sampleSize, funcMaxInputs * sizeof(int)); be > >>>>>>> preferred to the plain for-loops? > >>>>>>> > >>>>>>>> Yes. it's what the other functions use, right? > >>>>>>> > >>>>>>> Regards, Adam. > >>>>>>> > >>>>>>> P.S.: By the way, is there a reason to use the > >>>>>>> signature Function(const Function*) for the "what would > >>>>>>> be a copy constructor" instead of "Function(const > >>>>>>> Function&)"? > >>>>>>> > >>>>>>>> No clue, ask the guy that wrote them (hint, he's not > >>>>>>>> around) > >>>>>>>> > >>>>>>>> Cheers, Albert > >>>>>>> > >>>>>>> On 13.11.2012 23:28, Albert Astals Cid wrote: > >>>>>>>>>> El Dilluns, 27 d'agost de 2012, a les 22:31:13, > >>>>>>>>>> Albert Astals Cid > >>>>>>>>>> > >>>>>>>>>> va escriure: > >>>>>>>>>>> El Dilluns, 27 d'agost de 2012, a les > >>>>>>>>>>> 08:29:40, Thomas Freitag va > >>>>>>>>>>> > >>>>>>>>>>> escriure: > >>>>>>>>>>>> On 27.08.2012 00:56, Albert Astals Cid > >>>>>>>>>>>> > >>>>>>>>>>>> wrote: > >>>>>>>>>>>>> El Diumenge, 26 d'agost de 2012, a les > >>>>>>>>>>>>> 15:48:37, He Liu va > >>>>>>>>>>>>> > >>>>>>>>>>>>> escriure: > >>>>>>>>>>>>>>>> 5. vtable pointer will be > >>>>>>>>>>>>>>>> overwritten Function.cc:422:10: > >>>>>>>>>>>>>>>> warning: destination for this > >>>>>>>>>>>>>>>> 'memcpy' call is a pointer to dynamic > >>>>>>>>>>>>>>>> class 'SampledFunction'; vtable > >>>>>>>>>>>>>>>> pointer will be overwritten > >>>>>>>>>>>>>>>> [-Wdynamic-class-memaccess] > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> memcpy(this, func, > >>>>>>>>>>>>>>>> sizeof(SampledFunction)); ~~~~~~ ^ > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Function.cc:422:10: note: explicitly > >>>>>>>>>>>>>>>> cast the pointer to silence this > >>>>>>>>>>>>>>>> warning > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> At least categrory 5. sound serious > >>>>>>>>>>>>>>>> to me, I would never have copied > >>>>>>>>>>>>>>>> instances of C++ objects in that way, > >>>>>>>>>>>>>>>> because it depends on the compiler > >>>>>>>>>>>>>>>> and the class if this causes problems > >>>>>>>>>>>>>>>> on runtime, s. i.e. > >>>>>>>>>>>>>>>> http://weseetips.com/tag/afx_zero_init_object/, > > > > Note this is memset-ing to 0, not memcpy-ing a > > > >>>>>>>>>>>>>>> class to itself. To be honest i agree > >>>>>>>>>>>>>>> memcpy'in a SampledFunction to a > >>>>>>>>>>>>>>> SampledFunction is ugly, but i fail to > >>>>>>>>>>>>>>> see why it would not work. > >>>>>>>>>>>> > >>>>>>>>>>>> It works, at least with the actual used > >>>>>>>>>>>> compilers. But it works only, because the > >>>>>>>>>>>> allocated members of SampledFunction are > >>>>>>>>>>>> overwriten after doing the memcpy. And this > >>>>>>>>>>>> behaviour makes it just more ugly in my eyes. > >>>>>>>>>>>> And this is the same with the other memcpy's > >>>>>>>>>>>> in ExponentialFunction, StitchingFunction and > >>>>>>>>>>>> PostScriptFunction. And it will work till > >>>>>>>>>>>> such time as everybody who changes the class > >>>>>>>>>>>> will not forget to do it in the same way. So > >>>>>>>>>>>> if You are not willing to change it (or let > >>>>>>>>>>>> somebody else change it, I know, never change > >>>>>>>>>>>> running code), we could use the hint: > >>>>>>>>>>>> explicitly cast the pointer to silence this > >>>>>>>>>>>> warning. > >>>>>>>>>>> > >>>>>>>>>>> I'm fine with someone fixing the code. Less > >>>>>>>>>>> warnings is always good, but for master, > >>>>>>>>>>> there's no need to introduce a pontential > >>>>>>>>>>> breaker just because clang complains. > >>>>>>>>>> > >>>>>>>>>> So at the end noone volunteered to re-rewite > >>>>>>>>>> those functions not to use memcpy over classes > >>>>>>>>>> with virtuals? > >>>>>>>>>> > >>>>>>>>>> Cheers, Albert > >>>>>>>>>> > >>>>>>>>>>> Cheers, Albert > >>>>>>>>>>> > >>>>>>>>>>>> Cheers, Thomas > >>>>>>>>>>>> > >>>>>>>>>>>>>> Hi Albert, > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> :-) > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> A pointer of type SampleFunction* could > >>>>>>>>>>>>>> be pointing to an instance of a > >>>>>>>>>>>>>> SampleFunction sub-class, which has > >>>>>>>>>>>>>> different vtable contents. > >>>>>>>>>>>>> > >>>>>>>>>>>>> No, it could not, SampledFunction does not > >>>>>>>>>>>>> have any childs and the function doing that > >>>>>>>>>>>>> memcopy is private anyway. > >>>>>>>>>>>>> > >>>>>>>>>>>>>> As a result, one could construct a > >>>>>>>>>>>>>> SampleFunction with > >>>>>>>>>>>>>> SampleFunction(SampleFunction *) using a > >>>>>>>>>>>>>> pointer to a sub-class instance, and > >>>>>>>>>>>>>> overwrite the SampleFunction's vtable > >>>>>>>>>>>>>> address with the sub-class's vtable > >>>>>>>>>>>>>> address. > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> I am not sure if it will lead to any > >>>>>>>>>>>>>> bugs/vulnerabilities in this case, but it > >>>>>>>>>>>>>> is not safe practice in general. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Sure, i never said it was. I'm just saying > >>>>>>>>>>>>> i don't see why it would not work in our > >>>>>>>>>>>>> case. > >>>>>>>>>>>>> > >>>>>>>>>>>>>> Since the vtable structure depends on how > >>>>>>>>>>>>>> the compiler is implemented, memcpy or > >>>>>>>>>>>>>> memset on object pointers will generally > >>>>>>>>>>>>>> lead to undefined behaviors. > >>>>>>>>>>>>> > >>>>>>>>>>>>> I'm far from a compiler expert, but one > >>>>>>>>>>>>> would hope that for a given class the > >>>>>>>>>>>>> compiler stores always the "stuff" in the > >>>>>>>>>>>>> same order in memory, so again, i fail to > >>>>>>>>>>>>> see why this should fail in our case. > >>>>>>>>>>>>> > >>>>>>>>>>>>> Cheers, > >>>>>>>>>>>>> > >>>>>>>>>>>>> Albert > >>>>>>>>>>>>> > >>>>>>>>>>>>>> Thanks. > >>>>>>>>>>>>>> _______________________________________________ > > > > poppler mailing list [email protected] > > > >>>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler > > _______________________________________________ poppler > > >>>>>>>>>>>>> mailing list [email protected] > >>>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler > > > > . > > > >>>>>>>>>>>> _______________________________________________ > > poppler mailing list [email protected] > > >>>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler > > > > _______________________________________________ poppler > > > >>>>>>>>>>> mailing list [email protected] > >>>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler > > > > _______________________________________________ poppler > > > >>>>>>>>>> mailing list [email protected] > >>>>>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler > > _______________________________________________ poppler > > >>>>>>> mailing list [email protected] > >>>>>>> http://lists.freedesktop.org/mailman/listinfo/poppler > >>>> > >>>> _______________________________________________ poppler > >>>> mailing list [email protected] > >>>> http://lists.freedesktop.org/mailman/listinfo/poppler > > > > _______________________________________________ poppler mailing > > list [email protected] > > http://lists.freedesktop.org/mailman/listinfo/poppler > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2.0.19 (GNU/Linux) > > iQEcBAEBAgAGBQJQqyvUAAoJEPSSjE3STU34yhgH/3LnXGrOTPOZHOSCpi8/u4gs > Tku7C53A9bmapic70PMEZuKFsq2D9j+e1LsM4xx0lyKiu/vGXLZjGnOrAlbO2BTC > xKw7NfeIm7BJG7NuiEahbG3qKFVpNuxb50s/X1m7qsPOXxzJd2mbJmV17NSW/Je0 > EgubzRE+JBpnRXAqDiU22Lf7HsfKaAzcN1qRD6Rh6P7utvS5CRNJBlU0VMucNnEq > 6i/hFdeKyN6gewu9hCkAGnM0fsYMuHaxbjg2RjqxTNfDwlvkV07WHn0xYPcY3HbD > KYKOm9ZxG+eKh66AH4s6jjPHRH7hTHBTJnziNOU/rCZK1PbyH0y0QaaSNzNEXLM= > =TGDM > -----END PGP SIGNATURE----- _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
