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 07:19:28 GMT
On Mon, Jan 01, 2001 at 10:06:33PM -0800, rbb@covalent.net wrote:
>...
> > > 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.
> > 
> > APR_DECLARE_POOL_ACCESSOR(file)
> > APR_IMPLEMENT_POOL_ACCESSOR(file)
> > 
> > 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.

No... it would follow the hook style. The DECLARE is used in a header, and
the IMPLEMENT is used in a .c file.

Let's get concrete here:

#define APR_DECLARE_POOL_ACCESSOR(name) \
    APR_DECLARE(apr_pool_t *) apr_##name##_get_pool(apr_##name##_t *ob);

#define APR_IMPLEMENT_POOL_ACCESSOR(name) \
    APR_DECLARE(apr_pool_t *) apr_##name##_get_pool(apr_##name##_t *ob) \
    { return ob->pool; }

This creates type-safe accessors quite easily. It also meets the need for
getting at a pool, and meets your point about the hooks: "without affecting
the amount of duplicated code or maintainability."

>...
> 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.

When you show me that we have a performance problem, then I'll change my
mind. Until then, this is a "premature optimization" at the cost of clarity,
robustness/safeness, and maintainability.

>...
> > > 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

I disagree, but am fine with agreeing-to-disagree.

>...
> > > > 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 quite fine with adding a STATUS entry to discuss final resolution. But
that resolution should be typesafe and should be completed before the next
release.
[ done ]

Cheers,
-g

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

Mime
View raw message