You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Garrett Rooney <ro...@electricjellyfish.net> on 2005/08/16 04:24:29 UTC
APR_ARRAY_FOO convenience macros
So back in Dec 2003 Sander Striker suggested [1] adding Subversion's
macros for manipulating apr arrays (APR_ARRAY_IDX, which automates the
casting needed to access entries in the array, and APR_ARRAY_PUSH, which
automates the casting needed to add entries to the array) to
apr_tables.h. Roy Fielding commented [2] that he felt the macros tied
the interface to the implementation too much, and Greg Stein replied [3]
that they had to be done as macros, not functions, because of the need
to pass a type name for the cast. Conversation sort of died off there.
And here we are, years later, and I still find myself copying these
macros into each and ever program I write that uses APR because without
them I'm simply unable to use the apr array interface without looking up
how to do it each and every time. Does anyone have any reason we can't
just add them? The interface for these data structures is wide open, we
can't change it without massive changes to each and every caller anyway,
can we please just include the macros so apr arrays aren't such a
collosal pain in the ass to actually use?
For the curious, the actual macros are here:
http://svn.collab.net/repos/svn/trunk/subversion/include/svn_types.h
-garrett (who just screwed up the casting to access an apr array in a
patch to mod_smtpd and would prefer to never have that happen again)
[1]
http://mail-archives.apache.org/mod_mbox/apr-dev/200312.mbox/%3c1071252440.1568.7093.camel@localhost.localdomain%3e
[2]
http://mail-archives.apache.org/mod_mbox/apr-dev/200312.mbox/%3cBA3254D8-2D04-11D8-A9BB-000393753936@gbiv.com%3e
[3]
http://mail-archives.apache.org/mod_mbox/apr-dev/200312.mbox/%3c20031212180702.B27042@lyra.org%3e
Re: APR_ARRAY_FOO convenience macros
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Aug 15, 2005 at 07:24:29PM -0700, Garrett Rooney wrote:
> So back in Dec 2003 Sander Striker suggested [1] adding Subversion's
> macros for manipulating apr arrays (APR_ARRAY_IDX, which automates the
> casting needed to access entries in the array, and APR_ARRAY_PUSH, which
> automates the casting needed to add entries to the array) to
> apr_tables.h. Roy Fielding commented [2] that he felt the macros tied
> the interface to the implementation too much, and Greg Stein replied [3]
> that they had to be done as macros, not functions, because of the need
> to pass a type name for the cast. Conversation sort of died off there.
+1 to adding these.
joe
Re: APR_ARRAY_FOO convenience macros
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Brian W. Fitzpatrick wrote:
>
> On Aug 17, 2005, at 10:47 PM, Branko Čibej wrote:
>
>> Cliff Woolley wrote:
>>
>>>
>>> I'm fine with adding these. +1
>>>
>>>
>> +1, in case we need another vote. I know we'd go barmy in SVN without
>> these macros.
>
>
> +1 indeed.
Since the overwhelming majority appears to be in favor of it, I
committed the change in revision 233287.
-garrett
Re: APR_ARRAY_FOO convenience macros
Posted by "Brian W. Fitzpatrick" <fi...@red-bean.com>.
On Aug 17, 2005, at 10:47 PM, Branko Čibej wrote:
> Cliff Woolley wrote:
>
>
>> On Wed, 17 Aug 2005, Joe Orton wrote:
>>
>>
>>
>>> On Tue, Aug 16, 2005 at 01:49:55AM -0500, William Rowe wrote:
>>>
>>>
>>>> At 09:24 PM 8/15/2005, Garrett Rooney wrote:
>>>>
>>>>
>>>>> So back in Dec 2003 Sander Striker suggested [1] adding
>>>>> Subversion's macros for manipulating apr arrays (APR_ARRAY_IDX,
>>>>> which automates the casting needed to access entries in the
>>>>> array, and APR_ARRAY_PUSH, which automates the casting needed
>>>>> to add entries to the array) to apr_tables.h. Roy Fielding
>>>>> commented [2] that he felt the macros tied the interface to the
>>>>> implementation too much, and Greg Stein replied [3] that they
>>>>> had to be done as macros, not functions, because of the need to
>>>>> pass a type name for the cast.
>>>>>
>>>>>
>>>> IIUC, these are not typesafe?
>>>>
>>>>
>>> APR arrays are fundamentally not type-safe object stores.
>>>
>>> In case it's not clear, these are the macros in question:
>>>
>>> /** index into an apr_array_header_t */
>>> #define APR_ARRAY_IDX(ary,i,type) (((type *)(ary)->elts)[i])
>>>
>>> /** easier array-pushing syntax */
>>> #define APR_ARRAY_PUSH(ary,type) (*((type *)apr_array_push (ary)))
>>>
>>>
>>
>> I'm fine with adding these. +1
>>
>>
> +1, in case we need another vote. I know we'd go barmy in SVN
> without these macros.
+1 indeed.
-Fitz, who had to look up "barmy"
PS http://www.answers.com/barmy&r=67
Re: APR_ARRAY_FOO convenience macros
Posted by Branko Čibej <br...@xbc.nu>.
Cliff Woolley wrote:
>On Wed, 17 Aug 2005, Joe Orton wrote:
>
>
>
>>On Tue, Aug 16, 2005 at 01:49:55AM -0500, William Rowe wrote:
>>
>>
>>>At 09:24 PM 8/15/2005, Garrett Rooney wrote:
>>>
>>>
>>>>So back in Dec 2003 Sander Striker suggested [1] adding Subversion's macros for manipulating apr arrays (APR_ARRAY_IDX, which automates the casting needed to access entries in the array, and APR_ARRAY_PUSH, which automates the casting needed to add entries to the array) to apr_tables.h. Roy Fielding commented [2] that he felt the macros tied the interface to the implementation too much, and Greg Stein replied [3] that they had to be done as macros, not functions, because of the need to pass a type name for the cast.
>>>>
>>>>
>>>IIUC, these are not typesafe?
>>>
>>>
>>APR arrays are fundamentally not type-safe object stores.
>>
>>In case it's not clear, these are the macros in question:
>>
>>/** index into an apr_array_header_t */
>>#define APR_ARRAY_IDX(ary,i,type) (((type *)(ary)->elts)[i])
>>
>>/** easier array-pushing syntax */
>>#define APR_ARRAY_PUSH(ary,type) (*((type *)apr_array_push (ary)))
>>
>>
>
>I'm fine with adding these. +1
>
>
+1, in case we need another vote. I know we'd go barmy in SVN without
these macros.
-- Brane
Re: APR_ARRAY_FOO convenience macros
Posted by Cliff Woolley <jw...@virginia.edu>.
On Wed, 17 Aug 2005, Joe Orton wrote:
> On Tue, Aug 16, 2005 at 01:49:55AM -0500, William Rowe wrote:
> > At 09:24 PM 8/15/2005, Garrett Rooney wrote:
> > >So back in Dec 2003 Sander Striker suggested [1] adding Subversion's macros for manipulating apr arrays (APR_ARRAY_IDX, which automates the casting needed to access entries in the array, and APR_ARRAY_PUSH, which automates the casting needed to add entries to the array) to apr_tables.h. Roy Fielding commented [2] that he felt the macros tied the interface to the implementation too much, and Greg Stein replied [3] that they had to be done as macros, not functions, because of the need to pass a type name for the cast.
> >
> > IIUC, these are not typesafe?
>
> APR arrays are fundamentally not type-safe object stores.
>
> In case it's not clear, these are the macros in question:
>
> /** index into an apr_array_header_t */
> #define APR_ARRAY_IDX(ary,i,type) (((type *)(ary)->elts)[i])
>
> /** easier array-pushing syntax */
> #define APR_ARRAY_PUSH(ary,type) (*((type *)apr_array_push (ary)))
I'm fine with adding these. +1
--Cliff
Re: APR_ARRAY_FOO convenience macros
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Aug 16, 2005 at 01:49:55AM -0500, William Rowe wrote:
> At 09:24 PM 8/15/2005, Garrett Rooney wrote:
> >So back in Dec 2003 Sander Striker suggested [1] adding Subversion's macros for manipulating apr arrays (APR_ARRAY_IDX, which automates the casting needed to access entries in the array, and APR_ARRAY_PUSH, which automates the casting needed to add entries to the array) to apr_tables.h. Roy Fielding commented [2] that he felt the macros tied the interface to the implementation too much, and Greg Stein replied [3] that they had to be done as macros, not functions, because of the need to pass a type name for the cast.
>
> IIUC, these are not typesafe?
APR arrays are fundamentally not type-safe object stores.
In case it's not clear, these are the macros in question:
/** index into an apr_array_header_t */
#define APR_ARRAY_IDX(ary,i,type) (((type *)(ary)->elts)[i])
/** easier array-pushing syntax */
#define APR_ARRAY_PUSH(ary,type) (*((type *)apr_array_push (ary)))
joe
Re: APR_ARRAY_FOO convenience macros
Posted by Greg Marr <gr...@alum.wpi.edu>.
At 12:08 PM 8/16/2005, Garrett Rooney wrote:
>William A. Rowe, Jr. wrote:
>>At 09:24 PM 8/15/2005, Garrett Rooney wrote:
>>
>>>So back in Dec 2003 Sander Striker suggested [1] adding
>>>Subversion's macros for manipulating apr arrays (APR_ARRAY_IDX,
>>>which automates the casting needed to access entries in the array,
>>>and APR_ARRAY_PUSH, which automates the casting needed to add
>>>entries to the array) to apr_tables.h. Roy Fielding commented [2]
>>>that he felt the macros tied the interface to the implementation
>>>too much, and Greg Stein replied [3] that they had to be done as
>>>macros, not functions, because of the need to pass a type name for
>>>the cast.
>>
>>IIUC, these are not typesafe?
>>See the apr optional function and hook apis for true typesafe
>>casts.
>
>I must be missing something, but I don't see how these macros could
>be implemented in a more typesafe way. Could you elaborate?
>
>The hooks code appears to be a means to store arbitrary functions
>and then retrieve them later, are you suggesting that separate
>functions for manipulating each type of array be created dynamically
>or something? I guess I'm just failing to see how such a thing
>would really work in practice, or I'm just misunderstanding what
>you're suggesting.
True, they're not typesafe, because you can put the wrong type in the
macro. The big difference is what was seen in the broken commit,
doing the casts yourselves hides any errors you might make by
accessing the wrong element of the array structure. The macros only
have to get the casts right once. Not using the macros means that
every place in the code needs to get the casts right.
Re: APR_ARRAY_FOO convenience macros
Posted by Ben Laurie <be...@algroup.co.uk>.
(Reviving a very old thread)
Garrett Rooney wrote:
> William A. Rowe, Jr. wrote:
>
>> Yes, but (especially registered functions) allow you to set
>> up an argument type/argument list by casting a function ptr,
>> rather than the actual data object. This means that every
>> invocation of APR_FN_type_FOO will accept only a certain type,
>> enforcing typesafety.
>>
>> Ben went to the trouble of figuring it out (brilliant!) so
>> I'd happily veto wrappers which don't enforce type safety.
>> I don't want to return to the 1.3 mod_ssl days (of a new
>> fn for each arg list) after we have such an elegant solution
>> at hand :) Take some time to grok
>>
>>
>> http://svn.apache.org/repos/asf/apr/apr-util/trunk/include/apr_optional_hooks.h
>>
>>
>
> I've read apr_optional_hooks.h and looked at the underlying
> implementation, and I don't see how this gains us a whole lot. What it
> does seem to do is add complexity by requiring the user to declare and
> then retrieve their accessor functions, and on top of that it sure
> doesn't seem to be thread safe, what with its use of global variables in
> apr_hooks.c with no apparent locking.
>
> On the other hand, we've already seen these accessor macros used to
> great effect in a large codebase, and I know for a fact that they make
> it much much much easier to work with APR arrays. No, they do not solve
> all problems, but they do improve the situation without adding a great
> deal of complexity.
>
> If I'm getting the wrong impression, and it really is far simpler to
> make use of these APIs for the purpose of providing helper functions for
> the APR array interface than it seems at first glance, then could you
> please explain how, perhaps by sketching out how you'd see the interface
> being used.
I doubt its simpler to implement them, but the strength of the hook and
optional function implementation is that it ties the types of the
various pieces to the name of the function or hook. So, it doesn't rely
on you putting the correct type in every time you access. Or, more to
the point, code that relies on you noticing that you changed the type.
I suspect the same trick could be employed to make APR array access
typesafe by creating named accessor functions that enforce the types.
There may be other ways (precompilers work well here :-) that avoid
having to use functions, but given that many modern compilers can inline
functions, perhaps that's not an issue.
In fact, thinking about it, creating non-functional access methods
linked to names is trivial. See the attached program, which produces a
warning at the appropriate spot.
Or, in short, the difference between what I did and what you did is a
typedef :-)
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html http://www.thebunker.net/
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
Re: APR_ARRAY_FOO convenience macros
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
William A. Rowe, Jr. wrote:
> Yes, but (especially registered functions) allow you to set
> up an argument type/argument list by casting a function ptr,
> rather than the actual data object. This means that every
> invocation of APR_FN_type_FOO will accept only a certain type,
> enforcing typesafety.
>
> Ben went to the trouble of figuring it out (brilliant!) so
> I'd happily veto wrappers which don't enforce type safety.
> I don't want to return to the 1.3 mod_ssl days (of a new
> fn for each arg list) after we have such an elegant solution
> at hand :) Take some time to grok
>
> http://svn.apache.org/repos/asf/apr/apr-util/trunk/include/apr_optional_hooks.h
>
I've read apr_optional_hooks.h and looked at the underlying
implementation, and I don't see how this gains us a whole lot. What it
does seem to do is add complexity by requiring the user to declare and
then retrieve their accessor functions, and on top of that it sure
doesn't seem to be thread safe, what with its use of global variables in
apr_hooks.c with no apparent locking.
On the other hand, we've already seen these accessor macros used to
great effect in a large codebase, and I know for a fact that they make
it much much much easier to work with APR arrays. No, they do not solve
all problems, but they do improve the situation without adding a great
deal of complexity.
If I'm getting the wrong impression, and it really is far simpler to
make use of these APIs for the purpose of providing helper functions for
the APR array interface than it seems at first glance, then could you
please explain how, perhaps by sketching out how you'd see the interface
being used.
I'm especially confused at how you think this is heading to the bad old
days of a new function for each arg list, when my proposal requires
exactly two macros, and you don't ever have to declare more functions
when you want to work with more types of arrays because the type is
passed in when you invoke the macro. Now if you were arguing that
requiring the user to pass in the type is too error prone, I could see
that side of the argument, but even so I don't see how it's any more
error prone than the existing state of affairs.
-garrett
Re: APR_ARRAY_FOO convenience macros
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 11:08 AM 8/16/2005, Garrett Rooney wrote:
>William A. Rowe, Jr. wrote:
>>
>>IIUC, these are not typesafe?
>>See the apr optional function and hook apis for true typesafe
>>casts.
>
>I must be missing something, but I don't see how these macros could be implemented in a more typesafe way. Could you elaborate?
>
>The hooks code appears to be a means to store arbitrary functions and then retrieve them later, are you suggesting that separate functions for manipulating each type of array be created dynamically or something? I guess I'm just failing to see how such a thing would really work in practice, or I'm just misunderstanding what you're suggesting.
Yes, but (especially registered functions) allow you to set
up an argument type/argument list by casting a function ptr,
rather than the actual data object. This means that every
invocation of APR_FN_type_FOO will accept only a certain type,
enforcing typesafety.
Ben went to the trouble of figuring it out (brilliant!) so
I'd happily veto wrappers which don't enforce type safety.
I don't want to return to the 1.3 mod_ssl days (of a new
fn for each arg list) after we have such an elegant solution
at hand :) Take some time to grok
http://svn.apache.org/repos/asf/apr/apr-util/trunk/include/apr_optional_hooks.h
Bill
Re: APR_ARRAY_FOO convenience macros
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
William A. Rowe, Jr. wrote:
> At 09:24 PM 8/15/2005, Garrett Rooney wrote:
>
>>So back in Dec 2003 Sander Striker suggested [1] adding Subversion's macros
>>for manipulating apr arrays (APR_ARRAY_IDX, which automates the casting
>>needed to access entries in the array, and APR_ARRAY_PUSH, which automates
>>the casting needed to add entries to the array) to apr_tables.h. Roy
>>Fielding commented [2] that he felt the macros tied the interface to the
>>implementation too much, and Greg Stein replied [3] that they had to be
>>done as macros, not functions, because of the need to pass a type name for
>> the cast.
>
>
> IIUC, these are not typesafe?
>
> See the apr optional function and hook apis for true typesafe
> casts.
I must be missing something, but I don't see how these macros could be
implemented in a more typesafe way. Could you elaborate?
The hooks code appears to be a means to store arbitrary functions and
then retrieve them later, are you suggesting that separate functions for
manipulating each type of array be created dynamically or something? I
guess I'm just failing to see how such a thing would really work in
practice, or I'm just misunderstanding what you're suggesting.
-garrett
Re: APR_ARRAY_FOO convenience macros
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 09:24 PM 8/15/2005, Garrett Rooney wrote:
>So back in Dec 2003 Sander Striker suggested [1] adding Subversion's macros for manipulating apr arrays (APR_ARRAY_IDX, which automates the casting needed to access entries in the array, and APR_ARRAY_PUSH, which automates the casting needed to add entries to the array) to apr_tables.h. Roy Fielding commented [2] that he felt the macros tied the interface to the implementation too much, and Greg Stein replied [3] that they had to be done as macros, not functions, because of the need to pass a type name for the cast.
IIUC, these are not typesafe?
See the apr optional function and hook apis for true typesafe
casts.
Bill