You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stanley Gambarin <st...@cs.bu.edu> on 1997/07/22 17:35:07 UTC
code cleanup (2)
(a) The previous message regarding
#if defined (STUFF)
func()
#else
# define func () /* nop */
#endif
should have stated that above would improve readability of
the source code without affecting any functionality.
(b) Can someone please change the function prototypes
and definitions to contain either struct pool or pool, as mixing both
produces inconsistency and is not good (IMO - i never humble)
(c) Regarding module exports functions in alloc.[ch]
- copy_array_hdr() is not used anywhere and is highly
implementation dependant. In the fight for the abstraction, this
function should not be exported.
- note_cleanups_for_[fd|socket] should not be exported.
- pclosesocket() - is this function used anywhere at all ???
- note_subprocesses() - also should not be exported.
(d) Memory pool stuff
- pstrcat() will return a character (with undefined value) if
no arguments are specified. It should instead return NULL.
- all functions accepting pool *p as one of the parameters
do not check if p is NULL. Some developers may want to pass p=NULL
to allocate memory from the global pool. Either checks should be made
for NULLness or the behavior should be documented.
Stanley.
Re: code cleanup (2)
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 22 Jul 1997, Stanley Gambarin wrote:
>
>
> On Tue, 22 Jul 1997, Dean Gaudet wrote:
> >
> > > - note_cleanups_for_[fd|socket] should not be exported.
> >
> > Nope, it's an exported function used in lots of places. It's needed to
> > implement some cleanups for resources not implemented in alloc.c.
> >
> These functions are called from popenf() series of functions.
> IMO, the only interface to the module developers should be a call to
> popenf() function, not direct interface with the cleanup routines. Also,
> it would be a good idea to create a popensocket() routine to complement
> pclosesocket() routine.
grep note_cleanups http_main.c modules/proxy/*.c
popensocket() would do ... what? We'd need psocket(), paccept()...
> > > - note_subprocesses() - also should not be exported.
> >
> > The same, if a module forks something because it needs functionality
> > spawn_child doesn't offer ... it needs a way to request a cleanup of it.
> > I suppose the module could just write its own cleanup routine.
> >
> If module can not use Apache provided API, it is up to the module
> to handle the resources, not apache.
Sure and it handles it by telling alloc.c to cleanup the child... why
duplicate the code?
Dean
Re: code cleanup (2)
Posted by ra...@bellglobal.com.
> Disagreed. Apache believes in giving people enough rope to hang
> themselves, as long as they don't hang it, too. Besides, they're just
> useful tools that just call register_cleanup(), which is a perfectly good
> thing for a module to do.
Agreed. I am using cleanup functions in mod_php, and they have
nothing to do with popenf().
-Rasmus
Re: code cleanup (2)
Posted by Alexei Kosut <ak...@organic.com>.
On Tue, 22 Jul 1997, Stanley Gambarin wrote:
>
>
> On Tue, 22 Jul 1997, Dean Gaudet wrote:
> >
> > > - note_cleanups_for_[fd|socket] should not be exported.
> >
> > Nope, it's an exported function used in lots of places. It's needed to
> > implement some cleanups for resources not implemented in alloc.c.
> >
> These functions are called from popenf() series of functions.
> IMO, the only interface to the module developers should be a call to
> popenf() function, not direct interface with the cleanup routines. Also,
> it would be a good idea to create a popensocket() routine to complement
> pclosesocket() routine.
Disagreed. Apache believes in giving people enough rope to hang
themselves, as long as they don't hang it, too. Besides, they're just
useful tools that just call register_cleanup(), which is a perfectly good
thing for a module to do.
-- Alexei Kosut <ak...@organic.com>
Re: code cleanup (2)
Posted by Stanley Gambarin <st...@cs.bu.edu>.
On Tue, 22 Jul 1997, Dean Gaudet wrote:
>
> > - note_cleanups_for_[fd|socket] should not be exported.
>
> Nope, it's an exported function used in lots of places. It's needed to
> implement some cleanups for resources not implemented in alloc.c.
>
These functions are called from popenf() series of functions.
IMO, the only interface to the module developers should be a call to
popenf() function, not direct interface with the cleanup routines. Also,
it would be a good idea to create a popensocket() routine to complement
pclosesocket() routine.
> > - note_subprocesses() - also should not be exported.
>
> The same, if a module forks something because it needs functionality
> spawn_child doesn't offer ... it needs a way to request a cleanup of it.
> I suppose the module could just write its own cleanup routine.
>
If module can not use Apache provided API, it is up to the module
to handle the resources, not apache.
> > (d) Memory pool stuff
> > - pstrcat() will return a character (with undefined value) if no
> > arguments are specified. It should instead return NULL.
>
> Definately a bug.
>
Patch coming up by tomorrow unless someone beats me.
Stanley.
Re: code cleanup (2)
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 22 Jul 1997, Stanley Gambarin wrote:
> (a) The previous message regarding
> #if defined (STUFF)
> func()
> #else
> # define func () /* nop */
> #endif
> should have stated that above would improve readability of
> the source code without affecting any functionality.
Yeah it makes sense to me.
> (c) Regarding module exports functions in alloc.[ch]
> - copy_array_hdr() is not used anywhere and is highly
> implementation dependant. In the fight for the abstraction, this
> function should not be exported.
Hmm. Dunno. It's a "copy-on-write" operation, which should be easy to
abstract.
> - note_cleanups_for_[fd|socket] should not be exported.
Nope, it's an exported function used in lots of places. It's needed to
implement some cleanups for resources not implemented in alloc.c.
> - pclosesocket() - is this function used anywhere at all ???
The proxy.
> - note_subprocesses() - also should not be exported.
The same, if a module forks something because it needs functionality
spawn_child doesn't offer ... it needs a way to request a cleanup of it.
I suppose the module could just write its own cleanup routine.
> (d) Memory pool stuff
> - pstrcat() will return a character (with undefined value) if no
> arguments are specified. It should instead return NULL.
Definately a bug.
> - all functions accepting pool *p as one of the parameters
> do not check if p is NULL. Some developers may want to pass p=NULL
> to allocate memory from the global pool. Either checks should be made
> for NULLness or the behavior should be documented.
No, I don't want p == NULL to mean "the global pool". Stuff that uses
pconf now is carefully chosen. We don't want modules to have some easy
method of whacking things into a pool that is only clean on restarts. I'd
rather see it segfault for this... since it's possible a NULL pointer is
being passed in error. Consider a foo->pool element in some structure
which wasn't initialized.
Dean