----- Original Message -----
> Date: Fri, 19 Jun 2015 12:23:39 -0400
> From: anderson(a)redhat.com
> To: crash-utility(a)redhat.com
> Subject: Re: [Crash-utility] [PATCH v2] files: support dump file pages from
> its address space
>
> > > Here's what I suggest. Instead of using RADIX_TREE_DUMP, create a new
> > > flag
> > > RADIX_TREE_DUMP_CALLBACK, and you can store the callback function
> > > address inside
> > > the general purpose pc->curcmd_private member.
> >
> > I'm ok with use a different flag. Although I didn't find any source
codes
> > are
> > using RADIX_TREE_DUMP flag.
> >
> > But I had some concerns for storing callback address in
> > pc->curcmd_private.
> >
> > I noticed that the pc->curcmd_private is already used by other crash code
> > at
> > many locations.
>
> Right, but only within the context of specific commands, such as "irq -u",
"vm -M",
> "search", "struct -f" and "runq -c". Similar to using
GETBUF(), the pc->curcmd_private
> is only in effect within the period of time a particular command is running, and
> is cleared by restore_sanity() prior to the next "crash> " prompt.
> >
> > If someday people want to use the RADIX_TREE_DUMP_CALLBACK at that context,
> > it may cause the troubles.
>
> How? If they want to do so within the context of a different command, they
> can set the pc->curcmd_private field to their own function. That's what
> it's designed to be used for.
The do_radix_tree is a general purpose api, but if we wanted to use it in "irq
-u" "search"
command, we would add the limitations here. The pc->curcmd_private already used in
these
commands.
OK, yes, in that case, point taken...
>
> How about this: have your new RADIX_TREE_DUMP_CALLBACK option use the rtp->value
> (void *) member to store the callback function? As long as it's documented, I
would
> have no problem accepting that as well.
I think this is a good idea.
Below is my plan,
a. add the RADIX_TREE_DUMP_CB flag for do_radix_tree api.
b. Change the rtp definition, use union to replace original void * value definition.
I think pre-build extension should be able to work with new code changes.
The union definition could make api easy to be understand.
It may work with pre-existing extension modules, but if an extension module that uses
a radix_tree_pair structure gets re-compiled, it fails like this:
error: 'struct radix_tree_pair' has no member named 'value'
I should have recommended using the radix_tree_pair.value as my first suggestion,
but I was mistakenly thinking that your code was already using it. (although even
if if you were using it, the value pointer could still be used, for example, to point
to a data structure containing a array pointer and your callback function)
So please just cast the value member to the callback function.
Thanks,
Dave