El Dimecres, 14 de novembre de 2012, a les 18:48:26, Adam Reichold va escriure: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > 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 > > -----BEGIN PGP SIGNATURE----- > Version: GnuPG v2.0.19 (GNU/Linux) > > iQEcBAEBAgAGBQJQo9lqAAoJEPSSjE3STU34xZEIAL34RZqQK3gnFvZ7LVYLM6rf > lpSG1qn0P0G5h1qexVtx9gMxoaN4m2lKMS2N9eomNu3IwxgP5D7phwdSfJesCkDS > gd19EQ+oDtwKdeLdQMq+oM63mYzr6nyVUHlPSFtIO4Z1VBdZCMsowQnTsUe45UJ9 > HWzqzzwiX91OTh7ILZqpQ4UtgmjZRIZmGxJVOqJcAK+qdVh7HiYmCbjGKnZ+YVdp > NtW2jWTTg6GL+rEMQS1WTR1oDgfIPLgQWAdULezS3fsOQQvthUMlQuZQMgeG5cVR > QGbQIjKTLjw/TNo+oQ+5r/lLfHynFYp3VNuAv/LCIHMeGZANpfKaQeTaao2ecMk= > =fieh > -----END PGP SIGNATURE----- _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
