Re: [CVS] RPM: rpm/rpmio/ strtolocale.c
On Fri, Jun 15, 2007, Jeff Johnson wrote:
On Jun 15, 2007, at 12:09 PM, Ralf S. Engelschall wrote:
Ok, thanks for the hint.
You've likely seen
foo = xmalloc(...)
...
foo = _free(foo)
habits of mine. Feel free to suggest better. _free seemed
like a good idea at the time, but I've never really found
the right place to add the static inline in rpm's API.
[...]
Yes, I've seen that _free is spreaded in the sources in a rather
redundant way. What about the following in system.h and then replace all
foo = _free(foo); with just xfree(foo)?
#define xfree(p) \
do { \
if ((p) != NULL) { \
free((void *)(p)); \
(p) = NULL; \
} \
} while (0)
This way the free is not even inside a sub-function (helps in debugging)
and you don't have to add a _free function to such a lot of source
files. But perhaps I'm just overlooking something here...
Ralf S. Engelschall
[EMAIL PROTECTED]
www.engelschall.com
__
RPM Package Managerhttp://rpm5.org
Developer Communication Listrpm-devel@rpm5.org
Re: [CVS] RPM: rpm/rpmio/ strtolocale.c
On Jun 15, 2007, at 12:56 PM, Ralf S. Engelschall wrote:
On Fri, Jun 15, 2007, Jeff Johnson wrote:
On Jun 15, 2007, at 12:09 PM, Ralf S. Engelschall wrote:
Ok, thanks for the hint.
You've likely seen
foo = xmalloc(...)
...
foo = _free(foo)
habits of mine. Feel free to suggest better. _free seemed
like a good idea at the time, but I've never really found
the right place to add the static inline in rpm's API.
[...]
Yes, I've seen that _free is spreaded in the sources in a rather
redundant way. What about the following in system.h and then
replace all
foo = _free(foo); with just xfree(foo)?
#define xfree(p) \
do { \
if ((p) != NULL) { \
free((void *)(p)); \
(p) = NULL; \
} \
} while (0)
This way the free is not even inside a sub-function (helps in
debugging)
and you don't have to add a _free function to such a lot of source
files. But perhaps I'm just overlooking something here...
The reason I did not do that is because of the (p) = NULL; side effect.
foo = creator(...)
...
foo = destuctor(...)
is the more general than malloc trash-and-burn the pointer paradigm I
try to follow.
Actaully splint has warped my coding style in this rather mechanically
robotic fashion. The annotation on _free() is sufficient hint for
splint NULL tracings.
73 de Jeff
__
RPM Package Managerhttp://rpm5.org
Developer Communication Listrpm-devel@rpm5.org
Re: [CVS] RPM: rpm/rpmio/ strtolocale.c
On Fri, Jun 15, 2007, Jeff Johnson wrote:
Yes, I've seen that _free is spreaded in the sources in a rather
redundant way. What about the following in system.h and then replace all
foo = _free(foo); with just xfree(foo)?
#define xfree(p) \
do { \
if ((p) != NULL) { \
free((void *)(p)); \
(p) = NULL; \
} \
} while (0)
This way the free is not even inside a sub-function (helps in debugging)
and you don't have to add a _free function to such a lot of source
files. But perhaps I'm just overlooking something here...
The reason I did not do that is because of the (p) = NULL; side effect.
[...]
Can you be more specific, Jeff? What side-effect are you talking about?
Are you talking about the fact that the macro argument is evaluated
multiple times? That's right, but I hope you just plan to do plain
xfree(foo) calls where foo is a simple variable name and not things
like xfree(array[counter++]) or such obvious side-effect troubling
things. IMHO this drawback of potential side-effects is less horrible
than having dozend of _free definitions floating around, isn't it?
Ralf S. Engelschall
[EMAIL PROTECTED]
www.engelschall.com
__
RPM Package Managerhttp://rpm5.org
Developer Communication Listrpm-devel@rpm5.org
Re: [CVS] RPM: rpm/rpmio/ strtolocale.c
On Jun 15, 2007, at 1:45 PM, Ralf S. Engelschall wrote:
On Fri, Jun 15, 2007, Jeff Johnson wrote:
Yes, I've seen that _free is spreaded in the sources in a rather
redundant way. What about the following in system.h and then
replace all
foo = _free(foo); with just xfree(foo)?
#define xfree(p) \
do { \
if ((p) != NULL) { \
free((void *)(p)); \
(p) = NULL; \
} \
} while (0)
This way the free is not even inside a sub-function (helps in
debugging)
and you don't have to add a _free function to such a lot of source
files. But perhaps I'm just overlooking something here...
The reason I did not do that is because of the (p) = NULL; side
effect.
[...]
Can you be more specific, Jeff? What side-effect are you talking
about?
Are you talking about the fact that the macro argument is evaluated
multiple times? That's right, but I hope you just plan to do plain
xfree(foo) calls where foo is a simple variable name and not things
like xfree(array[counter++]) or such obvious side-effect troubling
things. IMHO this drawback of potential side-effects is less horrible
than having dozend of _free definitions floating around, isn't it?
Sorry for imprecise use of side-effect. I did not mean the
classic side effect usage of macro defines such as
xfree(p++)
which, duh, breaks your xfree define. I'm perfectly willing to be
punished
if I choose to code xfree(p++) or equivalent.
Perhaps hidden is the more accurate term to express why I did not
choose
an xfree() #define.
I could have designed around
void destructor()
of which xfree() is an example.
Instead for idiosyncratic reasons I chose
foo = destructor(foo);
and tried to follow that paradigm.
Meanwhile, most, at least the important, objects in rpm are refcounted
which is an entirely different usage case than malloc/free anyways.
No matter what, I'd love to stamp out the multiple static inline _free
() crapola.
system.h is per-rpm-build only, not installed or distributed.
rpmlib.h has a _free() definition, but as code spread to rpmdb - rpmio,
I had to invent new locations for _free()
One of the rpmio/*.h files is likely best place to instantiate _free.
_free is just another fetish, likely fear of public embarassment is
what stopped
me from doing years ago. I'll figger a place and get rid of the
multiple copies
of _free() this weekend.
73 de Jeff
__
RPM Package Managerhttp://rpm5.org
Developer Communication Listrpm-devel@rpm5.org
