[ktap] Re: [PATCH 1/3] symbol: replace find_symbol() by get_dso_symbols()
- From: Azat Khuzhin <a3at.mail@xxxxxxxxx>
- To: Jovi Zhangwei <jovi.zhangwei@xxxxxxxxx>
- Date: Thu, 14 Nov 2013 12:27:54 +0400
On Thu, Nov 14, 2013 at 6:37 AM, Jovi Zhangwei <jovi.zhangwei@xxxxxxxxx> wrote:
> Hi Azat,
Hi Jovi,
>
> Thanks your patches.
>
> On Thu, Nov 14, 2013 at 3:13 AM, Azat Khuzhin <a3at.mail@xxxxxxxxx> wrote:
>> Old find_symbol() just find symbol by name, the new one
>> get_dso_symbols() will return all symbols/notes.
>>
>> And user of this functions, will match name/pattern by itself.
>>
>> For now get_dso_symbols() used like find_symbol(), this behaviour will
>> be changed in next patches.
>>
>> Signed-off-by: Azat Khuzhin <a3at.mail@xxxxxxxxx>
>> ---
>> userspace/eventdef.c | 13 ++++++-
>> userspace/symbol.c | 105
>> +++++++++++++++++++++++++++++++++++++++++----------
>> userspace/symbol.h | 25 +++++++++---
>> 3 files changed, 117 insertions(+), 26 deletions(-)
>>
>> diff --git a/userspace/eventdef.c b/userspace/eventdef.c
>> index 748ca10..13703c6 100644
>> --- a/userspace/eventdef.c
>> +++ b/userspace/eventdef.c
>> @@ -333,6 +333,8 @@ static char *parse_events_resolve_symbol(char *event,
>> int type)
>> {
>> char *colon, *end, *tail, *binary, *symbol;
>> vaddr_t symbol_address;
>> + struct dso_symbol *symbols = NULL;
>> + size_t symbols_count, i;
>>
>> colon = strchr(event, ':');
>> if (!colon)
>> @@ -357,7 +359,16 @@ static char *parse_events_resolve_symbol(char *event,
>> int type)
>> symbol = strdup(colon + 1 /* skip ":" */);
>> }
>>
>> - symbol_address = find_symbol(binary, symbol, type);
>> + symbols_count = get_dso_symbols(&symbols, binary, type);
>> + for (i = 0; i < symbols_count; ++i) {
>> + if (strcmp(symbols[i].name, symbol))
>> + continue;
>> +
>> + symbol_address = symbols[i].addr;
>> + break;
>> + }
>> + free_dso_symbols(symbols, symbols_count);
>> +
>
> I have some concern on function get_dso_symbol, imagine one big binary
> have 10000+ symbols,
> then it's quite inefficient to allocate that symbols array, and
> probably not necessarily.
Yeah, that's realloc() is freaking me out
>
> How about use actor function? like below:
>
> int uprobe_symbol_actor(const char *binary, const char *symbol,
> unsigned long addr)
> {
> /* write to debufs */
> ...
> }
>
>
> nr = symbol_parse(binary, symbol, type, uprobe_symbol_actor);
> if (!nr) {
> /* no symbol found */
> }
>
> Then there don't need any dynamic memory allocation for symbols array.
> (probably this would be more proper when consider to parse function arguments
> and local variable in future, return a big symbols array cannot handle
> this feature easily)
But I didn't want to write to debugfs from symbol module.
Maybe callback to get_dso_symbol() will be more appropriate?
I was thinking about adding some "base" structure for symbols,
and using it allow to iterate throw symbols,
unfortunately this is not so simple.
I think for now better will be implement callbacks,
and after we will try to find more _correct_ solution.
What you think?
>
> Jovi
--
Respectfully
Azat Khuzhin
Other related posts:
