You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by David Reid <dr...@jetnet.co.uk> on 2001/04/18 19:47:21 UTC
apr_dso_make??
Remind me again, why aren't we just going to add apr_dso_os_put (or whatever
combination of the bits in the correct order is these days) and the _get
version to use apr_os_dso_t's? That *would* keep us in line with the rest
of the APR API wouldn't it?
david
Re: apr_dso_make??
Posted by David Reid <dr...@jetnet.co.uk>.
So basically we're arguing over the semantics of the name! I'll add the
code as _put and you guys can change the name to _fluffy_pink_rabbits if
it'll make you feel better.
david
----- Original Message -----
From: "Greg Stein" <gs...@lyra.org>
To: "APR Development List" <de...@apr.apache.org>
Sent: Thursday, April 19, 2001 12:05 AM
Subject: Re: apr_dso_make??
> On Wed, Apr 18, 2001 at 06:47:21PM +0100, David Reid wrote:
> > Remind me again, why aren't we just going to add apr_dso_os_put (or
whatever
> > combination of the bits in the correct order is these days) and the _get
> > version to use apr_os_dso_t's? That *would* keep us in line with the
rest
> > of the APR API wouldn't it?
>
> We discussed this a while back. The "put" forms actually don't make a lot
of
> sense. How do you make an empty FOO, so that you can put a native FOO into
> it? As a result, the useful form is to "make" a FOO using a native object.
>
> Hunh. Actually, it looks like the "put" forms are misnamed. They create an
> object, rather than putting a native handle into an existing object.
>
> Per the previous conversation, I'm +1 on renaming the "put" forms to
"make".
> The bootstrap problem of how to get a FOO means that "put" just isn't a
> useful abstraction. Not to mention it leads to confusion like what we just
> saw: you put into an existing apr_dso_handle_t, but put for files will
> create a new file object.
>
> Ooky.
>
> Oh, crap. I just looked at unix/locks.c::apr_os_lock_put(). It expects the
> user to do the whole "set to NULL before calling" thing. Damn, that style
> was a source of bugs to no end (recall that file_open worked that way for
a
> while and bit us all the time).
>
> I think it is confusing to do a "put" into an object. That can lead to
> resource leaks (what happens to the previous native FOO?). I think it
would
> be best for us to drop all the "put" stuff, and stick with "get" and
"make".
>
> Thoughts?
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
>
Re: apr_dso_make??
Posted by "William A. Rowe, Jr." <ad...@rowe-clan.net>.
From: "Greg Stein" <gs...@lyra.org>
Sent: Wednesday, April 18, 2001 6:05 PM
> We discussed this a while back. The "put" forms actually don't make a lot of
> sense. How do you make an empty FOO, so that you can put a native FOO into
> it? As a result, the useful form is to "make" a FOO using a native object.
>
> Hunh. Actually, it looks like the "put" forms are misnamed. They create an
> object, rather than putting a native handle into an existing object.
>
> Per the previous conversation, I'm +1 on renaming the "put" forms to "make".
> The bootstrap problem of how to get a FOO means that "put" just isn't a
> useful abstraction. Not to mention it leads to confusion like what we just
> saw: you put into an existing apr_dso_handle_t, but put for files will
> create a new file object.
Sounds like a plan, and get the cleaned up around the edges afterwards.
Make a note in renames_pending, please.
Bill
Re: apr_dso_make??
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Apr 18, 2001 at 06:47:21PM +0100, David Reid wrote:
> Remind me again, why aren't we just going to add apr_dso_os_put (or whatever
> combination of the bits in the correct order is these days) and the _get
> version to use apr_os_dso_t's? That *would* keep us in line with the rest
> of the APR API wouldn't it?
We discussed this a while back. The "put" forms actually don't make a lot of
sense. How do you make an empty FOO, so that you can put a native FOO into
it? As a result, the useful form is to "make" a FOO using a native object.
Hunh. Actually, it looks like the "put" forms are misnamed. They create an
object, rather than putting a native handle into an existing object.
Per the previous conversation, I'm +1 on renaming the "put" forms to "make".
The bootstrap problem of how to get a FOO means that "put" just isn't a
useful abstraction. Not to mention it leads to confusion like what we just
saw: you put into an existing apr_dso_handle_t, but put for files will
create a new file object.
Ooky.
Oh, crap. I just looked at unix/locks.c::apr_os_lock_put(). It expects the
user to do the whole "set to NULL before calling" thing. Damn, that style
was a source of bugs to no end (recall that file_open worked that way for a
while and bit us all the time).
I think it is confusing to do a "put" into an object. That can lead to
resource leaks (what happens to the previous native FOO?). I think it would
be best for us to drop all the "put" stuff, and stick with "get" and "make".
Thoughts?
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: apr_dso_make??
Posted by David Reid <dr...@jetnet.co.uk>.
FWIW, I've just committed the start of the apr_os_dso_ support as it was
missing and we probably should have had it in a while back. No code to back
up the headers and defines but I'll get some done on the plane to Denver
tomorrow. If I can get a network connection somewhere then I'll be able to
commit, but if I'm stuck with dial up then it'll ahve to wait until I get
home.
david
----- Original Message -----
From: "David Reid" <dr...@jetnet.co.uk>
To: "APR Development List" <de...@apr.apache.org>
Sent: Wednesday, April 18, 2001 7:51 PM
Subject: Re: apr_dso_make??
> Forgive my poor eyesight, but are we actually defining an apr_os_dso_t
> anywhere?
>
> A snippit from apr_portable.h
>
> #elif defined(__BEOS__)
> #include <kernel/OS.h>
>
> struct apr_os_lock_t {
> /* Inter proc */
> sem_id sem_interproc;
> int32 ben_interproc;
> /* Intra Proc */
> sem_id sem_intraproc;
> int32 ben_intraproc;
> };
>
> typedef int apr_os_file_t;
> typedef DIR apr_os_dir_t;
> typedef int apr_os_sock_t;
> typedef struct apr_os_lock_t apr_os_lock_t;
> typedef thread_id apr_os_thread_t;
> typedef thread_id apr_os_proc_t;
> typedef int apr_os_threadkey_t;
> typedef struct timeval apr_os_imp_time_t;
> typedef struct tm apr_os_exp_time_t;
>
> #else
> /* Any other OS should go above this one. This is the lowest common
> * denominator typedefs for all UNIX-like systems. :)
> */
>
> #ifdef NEED_UNION_SEMUN
> /* it makes no sense, but this isn't defined on solaris */
> union semun {
> long val;
> struct semid_ds *buf;
> ushort *array;
> };
> #endif
>
> struct apr_os_lock_t {
> #if APR_USE_SYSVSEM_SERIALIZE
> int crossproc;
> #elif APR_USE_FCNTL_SERIALIZE
> int crossproc;
> #elif APR_USE_PROC_PTHREAD_SERIALIZE
> pthread_mutex_t *crossproc;
> #elif APR_USE_FLOCK_SERIALIZE
> int crossproc;
> #else
> /* No Interprocess serialization, too bad. */
> #endif
> #if APR_HAS_THREADS
> /* If no threads, no need for thread locks */
> #if APR_USE_PTHREAD_SERIALIZE
> pthread_mutex_t *intraproc;
> #endif
> #endif
> };
>
> typedef int apr_os_file_t;
> typedef DIR apr_os_dir_t;
> typedef int apr_os_sock_t;
> typedef struct apr_os_lock_t apr_os_lock_t;
> #if APR_HAS_THREADS && APR_HAVE_PTHREAD_H
> typedef pthread_t apr_os_thread_t;
> typedef pthread_key_t apr_os_threadkey_t;
> #endif
> typedef pid_t apr_os_proc_t;
> typedef struct timeval apr_os_imp_time_t;
> typedef struct tm apr_os_exp_time_t;
> #endif
>
>
> Now I don't see that we've actually talked about apr_os_dso_t's at any
point
> in this file and I'm not sure it's even been added to APR and as such
that's
> probably what we should be doing! Then we simply follow the API we
already
> have don't we?
>
> Sorry if I'm missing something here but I've had a touch of sunstroke!!
>
> david
>
> (copied to doug first and then to the list as reply-all got me again!) :)
> ----- Original Message -----
> From: "Doug MacEachern" <do...@covalent.net>
> To: "David Reid" <dr...@jetnet.co.uk>
> Cc: "APR Development List" <de...@apr.apache.org>
> Sent: Wednesday, April 18, 2001 7:40 PM
> Subject: Re: apr_dso_make??
>
>
> > On Wed, 18 Apr 2001, David Reid wrote:
> >
> > > Remind me again, why aren't we just going to add apr_dso_os_put (or
> whatever
> > > combination of the bits in the correct order is these days) and the
_get
> > > version to use apr_os_dso_t's? That *would* keep us in line with the
> rest
> > > of the APR API wouldn't it?
> >
> > since apr_dso_handle_t is an incomplete type, we need to `make' an
> > apr_dso_handle_t:
> >
> > apr_dso_handle_t *dso;
> > status = apr_dso_make(p, &dso, (void *)native_handle); /* will alloc
*dso
> */
> >
> > i suppose it should be:
> >
> > status = apr_dso_make(p, &dso);
> > apr_os_dso_handle_put(dso, (void *)native_handle);
> >
> > and perhaps:
> > void *os_handle = apr_os_dso_handle_get(dso);
> >
>
Re: apr_dso_make??
Posted by David Reid <dr...@jetnet.co.uk>.
Forgive my poor eyesight, but are we actually defining an apr_os_dso_t
anywhere?
A snippit from apr_portable.h
#elif defined(__BEOS__)
#include <kernel/OS.h>
struct apr_os_lock_t {
/* Inter proc */
sem_id sem_interproc;
int32 ben_interproc;
/* Intra Proc */
sem_id sem_intraproc;
int32 ben_intraproc;
};
typedef int apr_os_file_t;
typedef DIR apr_os_dir_t;
typedef int apr_os_sock_t;
typedef struct apr_os_lock_t apr_os_lock_t;
typedef thread_id apr_os_thread_t;
typedef thread_id apr_os_proc_t;
typedef int apr_os_threadkey_t;
typedef struct timeval apr_os_imp_time_t;
typedef struct tm apr_os_exp_time_t;
#else
/* Any other OS should go above this one. This is the lowest common
* denominator typedefs for all UNIX-like systems. :)
*/
#ifdef NEED_UNION_SEMUN
/* it makes no sense, but this isn't defined on solaris */
union semun {
long val;
struct semid_ds *buf;
ushort *array;
};
#endif
struct apr_os_lock_t {
#if APR_USE_SYSVSEM_SERIALIZE
int crossproc;
#elif APR_USE_FCNTL_SERIALIZE
int crossproc;
#elif APR_USE_PROC_PTHREAD_SERIALIZE
pthread_mutex_t *crossproc;
#elif APR_USE_FLOCK_SERIALIZE
int crossproc;
#else
/* No Interprocess serialization, too bad. */
#endif
#if APR_HAS_THREADS
/* If no threads, no need for thread locks */
#if APR_USE_PTHREAD_SERIALIZE
pthread_mutex_t *intraproc;
#endif
#endif
};
typedef int apr_os_file_t;
typedef DIR apr_os_dir_t;
typedef int apr_os_sock_t;
typedef struct apr_os_lock_t apr_os_lock_t;
#if APR_HAS_THREADS && APR_HAVE_PTHREAD_H
typedef pthread_t apr_os_thread_t;
typedef pthread_key_t apr_os_threadkey_t;
#endif
typedef pid_t apr_os_proc_t;
typedef struct timeval apr_os_imp_time_t;
typedef struct tm apr_os_exp_time_t;
#endif
Now I don't see that we've actually talked about apr_os_dso_t's at any point
in this file and I'm not sure it's even been added to APR and as such that's
probably what we should be doing! Then we simply follow the API we already
have don't we?
Sorry if I'm missing something here but I've had a touch of sunstroke!!
david
(copied to doug first and then to the list as reply-all got me again!) :)
----- Original Message -----
From: "Doug MacEachern" <do...@covalent.net>
To: "David Reid" <dr...@jetnet.co.uk>
Cc: "APR Development List" <de...@apr.apache.org>
Sent: Wednesday, April 18, 2001 7:40 PM
Subject: Re: apr_dso_make??
> On Wed, 18 Apr 2001, David Reid wrote:
>
> > Remind me again, why aren't we just going to add apr_dso_os_put (or
whatever
> > combination of the bits in the correct order is these days) and the _get
> > version to use apr_os_dso_t's? That *would* keep us in line with the
rest
> > of the APR API wouldn't it?
>
> since apr_dso_handle_t is an incomplete type, we need to `make' an
> apr_dso_handle_t:
>
> apr_dso_handle_t *dso;
> status = apr_dso_make(p, &dso, (void *)native_handle); /* will alloc *dso
*/
>
> i suppose it should be:
>
> status = apr_dso_make(p, &dso);
> apr_os_dso_handle_put(dso, (void *)native_handle);
>
> and perhaps:
> void *os_handle = apr_os_dso_handle_get(dso);
>
Re: apr_dso_make??
Posted by Doug MacEachern <do...@covalent.net>.
On Wed, 18 Apr 2001, David Reid wrote:
> Remind me again, why aren't we just going to add apr_dso_os_put (or whatever
> combination of the bits in the correct order is these days) and the _get
> version to use apr_os_dso_t's? That *would* keep us in line with the rest
> of the APR API wouldn't it?
since apr_dso_handle_t is an incomplete type, we need to `make' an
apr_dso_handle_t:
apr_dso_handle_t *dso;
status = apr_dso_make(p, &dso, (void *)native_handle); /* will alloc *dso */
i suppose it should be:
status = apr_dso_make(p, &dso);
apr_os_dso_handle_put(dso, (void *)native_handle);
and perhaps:
void *os_handle = apr_os_dso_handle_get(dso);