apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: cvs commit: apr/include apr_general.h
Date Tue, 02 Jan 2001 03:14:28 GMT
On Mon, Jan 01, 2001 at 06:36:52PM -0800, rbb@covalent.net wrote:
> > > > Euh... this is dangerous as hell. Why do we need this?
> > > 
> > > A) why is this dangerous at all?
> > 
> > Casting a pointer to something else, and then presuming that a pool is at
> > the beginning of the structure, is just plain dangerous. The whole point of
> > being incomplete is to hide their organization; that assumption shouldn't be
> > made.
> 
> Every single APR incomplete type has a pool as the first field.  That is
> part of the design, and it is documented in the APRDesign doc.  This was
> purposefully done so that something like this macro could be implemented.

Heh. It's documented only cuz you put it in there today :-) Kind of like
saying, "well, I can write ugly code because I just updated the design
document to allow me to."

Just because you documented it, doesn't make it any safer.

> > > The only difference is that we are getting it from
> > > an incomplete type.
> > 
> > And that is the whole problem. It is incomplete -- the caller can't possibly
> > know the pool is at the start of the structure.
> 
> Yes they can.  If it's an APR type, and it is incomplete, then it has a
> pool as it's first field.  That is a part of the APR design, so if this
> isn't true, then that type is broken.

It is a non-typesafe hack.

We've spent quite a bit of time trying to increase type safety. Look at the
original hooks, the new hooks from Ben, the macros for the directives, etc.

> > > b) we need this so that we don't leak memory like a sieve.  I committed a
> > > use of this to the buckets code a few minutes ago.
> > 
> > Then write apr_file_get_pool(). Not a cast and an assumption.
> 
> That also means that we have to write apr_socket_get_pool and
> apr_lock_get_pool and apr_mmap_get_pool, etc.

Yup.

> We control the type
> definitions, and we control the macro.  This is very similar to what Dean
> did with the ap_iol code.

1) because it has been done before doesn't make it right
2) looking at ap_iol.h, I don't see what you are referring to. where did
   this casting stuff occur?


Ryan: this is really not a good idea because of the type safety stuff.
Please use accessor functions instead.

thx,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message