On Tue, Dec 7, 2010 at 12:31 AM, Peter Clifton <[email protected]> wrote: > On Mon, 2010-12-06 at 16:58 +1100, Stephen Ecob wrote: >> If you use (or intend to use) lib dmalloc with PCB you will find the >> following useful. >> >> I've uploaded a patch against current heaad to sourceforge, ID >> #3129279, described as follows: > > Looks useful, I will push it, however, please explain the following first: > > +# define MyCalloc(a, b, c) calloc((a) ? (a) : 1, (b) ? (b) : 1) > +# define MyMalloc(a, b) malloc((a) ? (a) : 1) > +# define MyRealloc(a, b, c) ((a) ? realloc(a, (b) ? (b) : 1) : malloc((b) ? > (b) : 1)) > > I don't see why we are adding a synonym with requesting zero bytes / > zero items, with requesting one item?
I preserved the semantics of the original functions, in case some part of the code relies on them. > Is there anywhere (BROKEN) in PCB which requests zero items / bytes, and > expects that call to work? Agreed, the semantics of the mymem allocation functions are foolish. A quick check of the parts of PCB that I'm currently using show no dependance on the 0==1 foolishness, but I haven't had time to test all 71 callers of these functions. > I'd rather commit: > > +# define MyCalloc(a, b, c) calloc((a), (b)) > +# define MyMalloc(a, b) malloc((a)) > +# define MyRealloc(a, b, c) realloc((a), (b)) > > NB: realloc will "do the right thing" if the previous pointer passed is > NULL, no need to check and swap for a malloc call. Yes, but note the (probably ancient) comment in mymem.c: * this is a save version because BSD doesn't support the * handling of NULL pointers in realloc() I don't know if that is still true for BSD, but we should preserve the workaround unless we are confident that all of PCB's supported platforms work correctly with realloc(NULL, ...) > One final nit, I'd prefer not to see the double negative in the header > files: > > #ifndef HAVE_LIBDMALLOC > ....NON D-Malloc stuff > #else > ....D-Malloc stuff > #endif > > > Could more readably be defined as: > > #ifdef HAVE_LIBDMALLOC > ....D-Malloc stuff > #else > ....NON D-Malloc stuff > #endif Sure, happy to fix that. > If you feel that your version is better as it matches the various other > #ifndef HAVE_LIBDMALLOC you added, then fair enough, I'll push as is > pending an answer on the #define questions. > > > FWIW, I'd love to see our own custom memory allocation routines die in a > fire. Lets plan to eventually replace them with malloc / realloc / free > calls, and this won't be an issue. I thoroughly agree :) > I did loose some respect for the dmalloc author(s) when I noticed on > their page they can't spell "Microsoft Windows" correctly. "Windoze" > > In general, valgrind (probably not even conceived of when dmalloc was > first written), is a much more useful tool for memory leak diagnosis, so > I wouldn't go _too_ far out of our way to make things dmalloc suitable. Sure. > You still won't see good back-traces for the r_tree allocations, for > example. (Given your usage and symptoms, I guess some r_trees are being > leaked in the autorouter code?) Just found the leak this morning, turns out it's yet another thing to dislike in mymem.c ;-) So regarding the patch: Dropping the (a) ? (a) : 1 foolishness would be cleaner, but could expose latent bugs in the 71 callers of the mymem allocators. I'm happy to proceed either way. What is your preference ? _______________________________________________ geda-user mailing list [email protected] http://www.seul.org/cgi-bin/mailman/listinfo/geda-user
