apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From r..@covalent.net
Subject Re: cvs commit: apr/include apr_general.h
Date Tue, 02 Jan 2001 06:06:33 GMT

> > > 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.
> > 
> > Yes, and those things could be implemented with type-safeness, without
> > affecting the amount of duplicated code, or maintainability.
> If that is your impression of the hooks, then I can give you a similar pair
> of macros for the incomplete types.
> or somesuch. Since I'm the one with the issue on type safety, I'd be quite
> happy to do this work.

This isn't possible.  The APR_IMPLEMENT_POOL_ACCESSOR function would need
to be in the header file that declares the underlying structure, which
means it won't be exported.

> > Creating 12
> > different functions (one per APR type) to get access to the pool, which is
> > garaunteed to be in the same place in each structure.  This is just adding
> > code that we need to maintain.  Granted, they are easy functions, but why
> > do we need or want 12 of them?  Plus, it is just gets more complex as we
> > add more types.
> Nah. These are brain-dead, zero-maintenance functions. There is no
> complexity or mental overhead with them.

It is 12 today, 13 tomorrow, and 14 in a month.  Who cares if they are
easy to maintain, the docs still need to be checked, they still need to be
tested, and we need to be sure we have one for each type.  To make matters
worse, we can't even be sure that these functions will be in-lined,
because we don't have in-lining on all platforms.  There is no reason for
this to be a function.

> > There are three types, ap_iol, iol_socket, and iol_file.  iol_socket and
> > iol_file are two types that rely on the fact that the first field in their
> > structures was an ap_iol.  Basically, you pass an ap_iol into any of the
> > underlying functions, and it gets cast into the correct type.  From there,
> Ah. That's why I didn't see what you were getting at. That situation is a
> "subclassing" scheme. Each subclass does the cast to access its data. That
> is still type-unsafe, but the casting occurs within the implementation in
> limited circumstances (rather than clients doing casts).

This is also a subclassing scheme, it is just a bit more obvious.  Every
APR class is a subclass of apr_t.  Yes, the cast is implemented in a
macro, which means that it can be discovered by smart programmers, so
what.  Clients don't do the cast.  The comments in the code come right out
and say the type should never be used outside of the macro.

> > > Ryan: this is really not a good idea because of the type safety stuff.
> > > Please use accessor functions instead.
> > 
> > I completely disagree.  This is a perfectly valid C operation.  I have
> It is valid, but I am saying that we should not use that technique for APR.
> I'm asking you to back out the change and use a different approach. As I
> mentioned, I am also willing to do the work.

I believe we are at an impass here.  You and I can argue this back and
forth for days, and we won't get anyplace.  I suggest we put this on hold
until somebody else chimes in with an opinion.  In the meantime, this
allows us to continue, so I am leaving it in place.  Without it, we will
need to put back in the NULL pools, which I am 100% against doing for any
length of time.

> I'm also worried about future work when the pool gets moved for some reason,
> or a new type is introduced without a pool, or it is introduced and the pool
> isn't at the beginning.
> All of the above situations are concerning me, and are easily avoided with
> accessor (typesafe) functions.

Yes, at the cost of being slower, and unnecessarily wordy.  Personally, as
an APR user, having a separate function for each APR type would really
bother me.  We are doing a simple operation here, we are asking the APR
type for it's pool.  Every APR type should have one, and it should be the
same function to get it from all of them.  I am asking for us to use a bit
of OO technique to solve a simple problem.

The cases you are worried about are cases we should never see.  There is
no reason to move the pool field, all APR types must have pools, that has
been a part of APR since the very beginning.  The only exceptions are the
incredibly simple types such as times.  All of those are complete types,
and we could add the pool if we wanted to (except for the simplest of time
types, which is a simple integer).  In fact, until tonight, the APR design
docs came right out and said that EVERY apr type needed a pool structure.


Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131

View raw message