You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Cliff Woolley <cl...@yahoo.com> on 2001/06/29 22:20:28 UTC

Re: cvs commit: apr/memory/unix apr_sms.c apr_sms_blocks.c apr_sms_std.c apr_sms_tracking.c

On 29 Jun 2001 dreid@apache.org wrote:

>   Log:
>   We add another phase to the sms creation/initialisation that
>   allows us to make calls that can/could use the sms and if we
>   haven't finished initialising it, it'll segafult due to the lack
>   of function pointers.
>
>   This came up when trying to get pools running as sms :)

I'll reiterate my desire to see us use a static const struct for the
function pointers in sms (a la the apr_bucket_type_t).  Not only would it
make it slightly faster to create an sms, it could help avoid these sorts
of problems.

I know, I know... if I want it done, I should just patch the damn thing.
I'll try to get to it this weekend.  =-)

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



RE: cvs commit: apr/memory/unix apr_sms.c apr_sms_blocks.c apr_sms_std.capr_sms_tracking.c

Posted by Sander Striker <st...@apache.org>.
> The problem with the static structure is that we loose the ability to have
> default allocators available and would have made no difference at all in
> this case as the lock creation is a different problem.  We'd still do the
> creation as
> 
> - calloc the memory
> - do the first part of the initialisation
> - add the functions
> - check what we have
> - do post init stuff
[...]

Exactly. Only you said it in a nicer way :)

Sander


Re: cvs commit: apr/memory/unix apr_sms.c apr_sms_blocks.c apr_sms_std.capr_sms_tracking.c

Posted by David Reid <dr...@jetnet.co.uk>.
The problem with the static structure is that we loose the ability to have
default allocators available and would have made no difference at all in
this case as the lock creation is a different problem.  We'd still do the
creation as

- calloc the memory
- do the first part of the initialisation
- add the functions
- check what we have
- do post init stuff

Bear in mind that some of the things we allocate (pool and lock) can't be
static so we'll still some dynamic allocations.

The problem with the locks is that if a pool == sms then when we try to
create the lock using the current, partially formed sms, then we segfault.
Doing it at a later phase (post_init) resolves this problem.

I have no objection to static structures but not sure they really gain us
very much and this is really an optimisation isn't it? :)  It's your time
however!

david

> On 29 Jun 2001 dreid@apache.org wrote:
>
> >   Log:
> >   We add another phase to the sms creation/initialisation that
> >   allows us to make calls that can/could use the sms and if we
> >   haven't finished initialising it, it'll segafult due to the lack
> >   of function pointers.
> >
> >   This came up when trying to get pools running as sms :)
>
> I'll reiterate my desire to see us use a static const struct for the
> function pointers in sms (a la the apr_bucket_type_t).  Not only would it
> make it slightly faster to create an sms, it could help avoid these sorts
> of problems.
>
> I know, I know... if I want it done, I should just patch the damn thing.
> I'll try to get to it this weekend.  =-)
>
> --Cliff
>
> --------------------------------------------------------------
>    Cliff Woolley
>    cliffwoolley@yahoo.com
>    Charlottesville, VA
>
>
>


RE: cvs commit: apr/memory/unix apr_sms.c apr_sms_blocks.c apr_sms_std.c apr_sms_tracking.c

Posted by Sander Striker <st...@apache.org>.
>>> I'll reiterate my desire to see us use a static const struct for the
>>> function pointers in sms (a la the apr_bucket_type_t).  Not
>>> only would it
>>> make it slightly faster to create an sms, it could help avoid
>>> these sorts of problems.
>>
>> This is not possible since we want to be able to set defaults in the
>> framework. With a const struct we are not able to do this.
>
> Why not?  If the sms module doesn't feel like implementing its own edition
> of one of the functions, it just sets the function pointer in its const
> struct to point to the default function.  Various bucket types do this
> sort of thing all the time.

Granted. That would be an option. I would however suggest to shove this
to the backburner until sms has matured a bit more. Until then I like the
idea of only changing the reference to a default function in one place. :)

> > And, no, it will not prevent these sort of problems. The framework has
> > to be able to do some allocating from the sms. In apr_sms_init() the
> > sms hasn't been initialized yet. Hence the need for a
> apr_sms_post_init()
> > function.
>
> I won't argue against apr_sms_post_init() too loudly.
>
> What I was imagining, though, was something along the lines of the
> apr_sms_foo_create() function passing a pointer to its static "type"
> structure into apr_sms_init().  So then apr_sms_init() could set the type
> pointer in the apr_sms_t and actually call apr_sms_malloc() if it needed,
> calling back into the module via the type structure.  That might be a
> stretch, I don't know.

Ah, that is what you meant... Well, if we are going to do that, we also
need to pass in the size of the specific sms structure (ie.
sizeof(blocks_sms)).
And then, you'd still have the same problem. You may have the functions
that are required to do the allocations, but the sms structure (which holds
the accounting information for a specific sms) has not yet been initialized
in apr_sms_init().

> --Cliff

Sander


RE: cvs commit: apr/memory/unix apr_sms.c apr_sms_blocks.c apr_sms_std.c apr_sms_tracking.c

Posted by Cliff Woolley <cl...@yahoo.com>.
On Sat, 30 Jun 2001, Sander Striker wrote:

> > I'll reiterate my desire to see us use a static const struct for the
> > function pointers in sms (a la the apr_bucket_type_t).  Not only would it
> > make it slightly faster to create an sms, it could help avoid these sorts
> > of problems.
>
> This is not possible since we want to be able to set defaults in the
> framework. With a const struct we are not able to do this.

Why not?  If the sms module doesn't feel like implementing its own edition
of one of the functions, it just sets the function pointer in its const
struct to point to the default function.  Various bucket types do this
sort of thing all the time.

> And, no, it will not prevent these sort of problems. The framework has
> to be able to do some allocating from the sms. In apr_sms_init() the
> sms hasn't been initialized yet. Hence the need for a apr_sms_post_init()
> function.

I won't argue against apr_sms_post_init() too loudly.

What I was imagining, though, was something along the lines of the
apr_sms_foo_create() function passing a pointer to its static "type"
structure into apr_sms_init().  So then apr_sms_init() could set the type
pointer in the apr_sms_t and actually call apr_sms_malloc() if it needed,
calling back into the module via the type structure.  That might be a
stretch, I don't know.

--Cliff

--------------------------------------------------------------
   Cliff Woolley
   cliffwoolley@yahoo.com
   Charlottesville, VA



RE: cvs commit: apr/memory/unix apr_sms.c apr_sms_blocks.c apr_sms_std.c apr_sms_tracking.c

Posted by Sander Striker <st...@apache.org>.
> On 29 Jun 2001 dreid@apache.org wrote:
> 
> >   Log:
> >   We add another phase to the sms creation/initialisation that
> >   allows us to make calls that can/could use the sms and if we
> >   haven't finished initialising it, it'll segafult due to the lack
> >   of function pointers.
> >
> >   This came up when trying to get pools running as sms :)
> 
> I'll reiterate my desire to see us use a static const struct for the
> function pointers in sms (a la the apr_bucket_type_t).  Not only would it
> make it slightly faster to create an sms, it could help avoid these sorts
> of problems.

This is not possible since we want to be able to set defaults in the
framework. With a const struct we are not able to do this.

And, no, it will not prevent these sort of problems. The framework has
to be able to do some allocating from the sms. In apr_sms_init() the
sms hasn't been initialized yet. Hence the need for a apr_sms_post_init()
function.

The situation was this [semi pseudo]:

apr_sms_xxx_create(apr_sms_t **sms, apr_sms_t *parent)
{
    ...
    new_sms = apr_sms_calloc(parent, size_of_this_sms);
    
    apr_sms_init(new_sms);

    ...
}

In the new situation we get:

apr_sms_xxx_create(apr_sms_t **sms, apr_sms_t *parent)
{
    ...
    new_sms = apr_sms_calloc(parent, size_of_this_sms);
    
    apr_sms_init(new_sms);

    ...

    apr_sms_post_init(new_sms);
}

The apr_sms_post_init() can be usefull for more than
one thing. I suggest moving the apr_sms_assert()
call into it, for example.

Sander