Hello, On 11/07/2013 10:30 AM, Joerg Sonnenberger wrote: > On Thu, Nov 07, 2013 at 08:11:09AM +0100, Kern Sibbald wrote: >> Can you tell me what bug the first patch fixes. In my opinion, >> the second memcpy() changed is not necessary and harmless. >> The changed code, in my opinion, is a bad construct. Is sizeof(*ctx) >> really properly defined when ctx is a struct. It would be *far* better >> to change it to sizeof(struct MD5Context). Even then, I don't >> really see the purpose of zeroing out the structure except perhaps >> for security reasons. > Yes, it is perfectly well defined. Expanding it to sizeof(struct > MD5Context) is a regression in the sense that duplicates the type > information and wouldn't be updated if the type ever changed. > I don't disagree that the zeroing is likely to going over board, but the > current use is just wrong code.
Yes, I agree that it is wrong, but as far as I know it is not a bug. I want to think about what should really be done: either zero the whole structure or remove the line entirely. If the current code really breaks something, please let me know. > >> The second patch that removes code, should not be removed as >> it will remove code that is used by some plugins and expose >> them to improperly allocated memory. It is true that it >> is not particularly (in general) a good idea to clear out memory >> allocated by new. However, in Bacula our C++ constructs permit >> doing so. > The way it is done violates C++ specs. Replacements for operator new and > delete must be global symbols, inline functions are just ignored. Given > that memory allocated with new will be initialised by the constructor, I > don't see the point. What plugins are using this? At least in the main > sources of 5.2.12, the header is only included by test-deltaseq-fd.c. I am not 100% sure what happens when the functions are inlined -- it is not something I usually do in any case. However, unless the current code crashes something, I see no reason to remove it, because it uses Bacula memory allocator rather than the system allocator, and thus is far safer and allows us to ensure that memory is not leaked. If I am not mistaken these routines are used by a number of the Bacula Systems plugins that will over time be committed to the community version. Best regards, Kern > > Joerg > ------------------------------------------------------------------------------ November Webinars for C, C++, Fortran Developers Accelerate application performance with scalable programming models. Explore techniques for threading, error checking, porting, and tuning. Get the most from the latest Intel processors and coprocessors. See abstracts and register http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk _______________________________________________ Bacula-devel mailing list Bacula-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/bacula-devel
