> 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
