--On Tuesday, April 15, 2003 7:39 PM -0400 Jeff Trawick <[EMAIL PROTECTED]> wrote:
You may have known that socket data is implemented as pool userdata in a
manner that prevented multiple sockets from the same pool being able to use
the same key for socket data. Another problem is that any cleanup specified
in the call to apr_socket_data_set() is run when the pool is cleaned up, not
when the socket is destroyed (in contradiction to the doc).
Err, yeah, why is it pool_userdata at all? I'm confused. Isn't using pool's userdata *way* overkill for this? Can't it just store a void* or something in its internal structure? Why does it have to live longer than the socket structure itself?
Are we really wanting to support multiple socket_data instances per socket? If a program wants that level, it seems it should just use the apr_pool_userdata directly rather than this unnecessary abstraction. KISS.
The context where I see this as useful is with utility routines (e.g., support libraries) that perform operations using the socket but don't have any leeway in how the app uses pools. (This is a natural for an iol implemented using a socket iol "enhancement" to APR, since there is no leeway on interface in order to work with all unmodified APR apps.)
Converting from socket userdata to pool userdata should be this "simple" (and it would work properly, unlike code depending on the current apr_socket_data_get/set that only works if there is one such socket in the pool):
old way to set: apr_socket_data_set(sock, "val", "mykey", NULL);
new way to set (once we add a pool accessor for sockets): apr_pool_t *sock_pool = apr_socket_pool_get(sock); char keybuf[22]; /* big enough for 123456789abdef0mykey\0 */
/* make key unique among all sockets in pool */
apr_snprintf(keybuf, sizeof keybuf, "%ppmykey", sock);
apr_pool_userdata_set("val", keybuf, NULL, sock_pool);old way to get: apr_socket_data_get(&val, "mykey", sock);
new way to get (needs pool accessor for sockets): apr_pool_t *sock_pool = apr_socket_pool_get(sock); char keybuf[22]; /* big enough for 123456789abdef0mykey\0 */
apr_snprintf(keybuf, sizeof keybuf, "%ppmykey", sock); apr_pool_userdata_get(&val, keybuf, sock_pool);
The snprintf trick to make the key unique among all sockets in the pool could be done in apr_socket_data_get/set but without having a bound on the size of the keys it requires dynamic storage allocation.
The overhead of building a unique key for every get/set operation is a giant concern to me.
Maybe there is a better idea for how to make the key unique without performing any expensive operations.
I can buy the use of having a pointer associated with the socket. It's common enough, but yeah, you can do the same with pool_userdata directly. I'd prefer tossing out the key part in apr_socket_data_*, but if you want to be cute and backwards-compat, yeah, the hash works as you described. I just question how many apps really need that. -- justin
shrug
Perhaps it would be preferable to have no socket user data at all rather than a limit of exactly one item... unclear...
