apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sander Striker" <stri...@samba-tng.org>
Subject RE: [PATCH] APR Stackable Memory System
Date Mon, 21 May 2001 08:35:06 GMT
> I've committed the first two patches.  While I was at it, I threw in a
> couple more styleguide cleanups that I spotted with the second commit.

Thx.

>> sms-abort.patch.2
>>     Adds an abort_fn member to the apr_sms_t. This
>> includes code calling the abort_fn, if present, on
>> APR_ENOMEM. The code to set the abort function is
>> not present yet. I added this to get sms one step
>> closer to apr pools.
>
> I'm wondering on this one if the abort function should have a prototype
> more like the one used by pools (ie, it should take just a status code
> rather than filename/linenum).

Ah, ok, I asked David about that before he left. I hope he doesn't mind
me quoting snippets here from what he said:

"
dreid>   Why not stick to the standard apr format of
dreid>
dreid>   apr_status_t abort_fn(char *sourcefile, int lineno);

striker> Ahh, there is such a function? I need to do a lot of digging into
APR.

[...]
striker> Btw, in apr_pools.h there is this:
striker> typedef int (*apr_abortfunc_t)(int retcode);

dreid>   I'll have to review the code further to give more optinions, but if
you
dreid>   try to stick with the standard format it'll make things better
dreid>   and also allows for stacking of layers with errors not being lost
:)

striker> Uhuh, agreed. I'll see what I can do. By the way, my intention was
striker> to reflect the offending caller of the apr_sms_malloc() call in
striker> sourcefile and lineno by doing something like:
striker>
striker> #define apr_sms_malloc(sms, size) \
striker>     apr_sms_malloc(sms, size, __FILE__, __LINE__)
striker>
striker> And to modify the apr_sms_malloc implementation accordingly. Would
this
striker> be usefull? I mean, this way you know which part of the code caused
the
striker> last memory allocation that made it abort.
striker>
striker> This ofcourse also goes for apr_sms_realloc and apr_sms_calloc.
"

>  I'm really only hesitant because I don't
> know for sure whether or not __FILE__ and __LINE__ are universally
> available symbols across all preprocessors.  Besides, knowing which file
> and line it aborted in isn't much help since it will always be within one
> of the memory functions, when we're more interested in which file and line
> *called* the memory allocation function that failed (I think).
>
> Thoughts?

Yes, I wanted to do something like this:

#define apr_sms_malloc(mem_sys, size) \
    apr_sms_malloc(mem_sys, size, __FILE__, __LINE__)

And change the prototype to:

APR_DECLARE(void *) apr_sms_malloc(apr_sms_t *sms, apr_size_t size,
                                   char *sourcefile, int lineno);

The only problem is that the memory systems themselves will need the
extra parameters too, otherwise it won't bubble up. So that's why I
didn't want to do this yet, because I'm not sure this is what is
wanted.
And ofcourse apr_sms_malloc is just the first, calloc and realloc
would have to be done too.
And then I start to wonder if we don't just want to do this to
all memory system functions (like free, reset, destroy), so someone
can implement a decent profiling/logging memory system.

As for the __FILE__ and __LINE__ symbols, I have yet to meet a
compiler package that doesn't support them. On the other hand I have
only seen a few: gcc (on Linux), VC++ (Winxxx), and a series of small
(mostly) unknow ones these and other platforms (Amiga). It is ANSI
standard... (I did not bring this up by the way :-).
Someone out there know of one that doesn't support them, please speak
up.

[...]
> Thanks,
> --Cliff

Sander


Mime
View raw message