[ktap] [PATCH 5/5] symbol: use actor instead of dynamically allocate array of symbols.
- From: Azat Khuzhin <a3at.mail@xxxxxxxxx>
- To: ktap@xxxxxxxxxxxxx
- Date: Fri, 15 Nov 2013 01:31:58 +0400
Like Jovi suggested, replace dynamic allocatins, by callback/actor.
I've renamed get_dso_symbols() -> read_dso_symbols(), since it really do
this.
read_dso_symbols() will call actor for every symbol/sdt note that it will
found.
And replace usage of get_dso_symbols() by read_dso_symbols() inn
parse_events_resolve_symbol() (uprobes)
This patch also fix some memory leaks (on failures), and drop some exit()
inside parse_events_resolve_symbol()
Signed-off-by: Azat Khuzhin <a3at.mail@xxxxxxxxx>
---
userspace/eventdef.c | 89 +++++++++++++++++++++++++++++++++-------------------
userspace/symbol.c | 70 +++++++----------------------------------
userspace/symbol.h | 26 +++++++--------
3 files changed, 78 insertions(+), 107 deletions(-)
diff --git a/userspace/eventdef.c b/userspace/eventdef.c
index df3225b..82e806d 100644
--- a/userspace/eventdef.c
+++ b/userspace/eventdef.c
@@ -368,66 +368,87 @@ static int parse_events_resolve_symbol(int fd, int seq,
char *event, int type)
return 0;
}
#else
+struct uprobe_base
+{
+ int fd;
+ int seq;
+ int ret_probe;
+ const char *event;
+ char *binary;
+ char *symbol;
+
+ size_t handled_events;
+};
+static void uprobe_symbol_actor(const char *name, vaddr_t addr, void *arg)
+{
+ struct uprobe_base *base = (struct uprobe_base *)arg;
+ int ret;
+
+ if (!strglobmatch(name, base->symbol))
+ return;
+
+ verbose_printf("symbol %s resolved to 0x%lx\n",
+ base->event, addr);
+ ret = write_uprobe_event(base->fd, base->ret_probe, base->seq,
+ base->binary, addr);
+
+ if (ret)
+ ++base->handled_events;
+}
+
static int parse_events_resolve_symbol(int fd, int seq, char *event, int type)
{
- char *colon, *end, *tail, *binary, *symbol;
- char *r;
+ char *colon, *end, *tail;
vaddr_t symbol_address;
- struct dso_symbol *symbols = NULL;
- size_t symbols_count, i;
- int ret = 0;
+ struct uprobe_base base = {
+ .fd = fd,
+ .seq = seq,
+ .event = event,
+
+ .handled_events = 0,
+ };
colon = strchr(event, ':');
if (!colon)
return 0;
- r = strstr(event, "%return");
+ base.ret_probe = !!strstr(event, "%return");
symbol_address = strtol(colon + 1 /* skip ":" */, NULL, 0);
- binary = strndup(event, colon - event);
+ base.binary = strndup(event, colon - event);
/**
* We already have address, no need in resolving.
*/
- if (symbol_address)
- return write_uprobe_event(fd, !!r, seq, binary, symbol_address);
+ if (symbol_address) {
+ int ret;
+ ret = write_uprobe_event(fd, base.ret_probe, seq,
+ base.binary, symbol_address);
+ free(base.binary);
+ return ret;
+ }
end = strpbrk(event, "% ");
if (end) {
tail = strdup(end);
- symbol = strndup(colon + 1 /* skip ":" */, end - 1 - colon);
+ base.symbol = strndup(colon + 1 /* skip ":" */, end - 1 -
colon);
} else {
tail = NULL;
- symbol = strdup(colon + 1 /* skip ":" */);
+ base.symbol = strdup(colon + 1 /* skip ":" */);
}
- symbols_count = get_dso_symbols(&symbols, binary, type);
- for (i = 0; i < symbols_count; ++i) {
+ read_dso_symbols(base.binary, type, uprobe_symbol_actor, (void *)&base);
- if (!strglobmatch(symbols[i].name, symbol))
- continue;
-
- symbol_address = symbols[i].addr;
-
- verbose_printf("symbol %s resolved to 0x%lx\n",
- event, symbol_address);
- ret = write_uprobe_event(fd, !!r, seq, binary, symbol_address);
- if (!ret)
- break;
- }
- free_dso_symbols(symbols, symbols_count);
-
- if (!ret) {
+ if (!base.handled_events) {
fprintf(stderr, "error: cannot find symbol %s in binary %s\n",
- symbol, binary);
- exit(EXIT_FAILURE);
+ base.symbol, base.binary);
}
- free(binary);
- free(symbol);
+ free(base.binary);
+ free(base.symbol);
if (tail)
free(tail);
- return ret;
+ return !!base.handled_events;
}
#endif
@@ -445,7 +466,9 @@ static int parse_events_add_uprobe(char *old_event, int
type)
return -1;
}
- parse_events_resolve_symbol(fd, event_seq, old_event, type);
+ ret = parse_events_resolve_symbol(fd, event_seq, old_event, type);
+ if (!ret)
+ return -1;
close(fd);
diff --git a/userspace/symbol.c b/userspace/symbol.c
index 789611e..7134db6 100644
--- a/userspace/symbol.c
+++ b/userspace/symbol.c
@@ -75,45 +75,12 @@ static vaddr_t find_load_address(Elf *elf)
return address;
}
-void free_dso_symbols(struct dso_symbol *symbols, size_t symbols_count)
-{
- size_t i;
- for (i = 0; i < symbols_count; ++i) {
- free(symbols[i].name);
- }
-
- free(symbols);
-}
-
-/**
- * libc have about ~2000 symbols.
- */
-#define SYMBOLS_COUNT 3000
-
-/**
- * realloc() and free() on failure
- *
- * TODO: allocation by chunks
- */
-static struct dso_symbol *
-realloc_symbols(struct dso_symbol *symbols, size_t symbols_count)
-{
- struct dso_symbol *new;
-
- new = realloc(symbols, sizeof(*symbols) * symbols_count);
-
- if (!new && symbols)
- free_dso_symbols(symbols, symbols_count);
-
- return new;
-}
-
static size_t elf_symbols(GElf_Shdr shdr)
{
return shdr.sh_size / shdr.sh_entsize;
}
-static size_t dso_symbols(struct dso_symbol **symbols, Elf *elf)
+static size_t dso_symbols(Elf *elf, symbol_actor actor, void *arg)
{
Elf_Data *elf_data = NULL;
Elf_Scn *scn = NULL;
@@ -138,24 +105,18 @@ static size_t dso_symbols(struct dso_symbol **symbols,
Elf *elf)
for (i = 0; i < elf_symbols(shdr); i++) {
char *name;
- struct dso_symbol symbol;
+ vaddr_t addr;
gelf_getsym(elf_data, i, &sym);
if (GELF_ST_TYPE(sym.st_info) != STT_FUNC)
continue;
- ++symbols_count;
- *symbols = realloc_symbols(*symbols, symbols_count);
- if (!*symbols) {
- symbols_count = 0;
- break;
- }
-
name = elf_strptr(elf, shdr.sh_link, sym.st_name);
- symbol.name = strdup(name);
- symbol.addr = sym.st_value - load_address;
- memcpy(&(*symbols)[symbols_count - 1], &symbol,
sizeof(symbol));
+ addr = sym.st_value - load_address;
+
+ actor(name, addr, arg);
+ ++symbols_count;
}
}
@@ -231,7 +192,7 @@ static const char *sdt_note_data(const Elf_Data *data,
size_t off)
return ((data->d_buf) + off);
}
-static size_t dso_sdt_notes(struct dso_symbol **symbols, Elf *elf)
+static size_t dso_sdt_notes(Elf *elf, symbol_actor actor, void *arg)
{
GElf_Ehdr ehdr;
Elf_Scn *scn = NULL;
@@ -266,7 +227,6 @@ static size_t dso_sdt_notes(struct dso_symbol **symbols,
Elf *elf)
(next = gelf_getnote(data, offset, &nhdr, &name_off,
&desc_off)) > 0;
offset = next) {
const char *name;
- struct dso_symbol symbol;
if (nhdr.n_namesz != sizeof(SDT_NOTE_NAME) ||
memcmp(data->d_buf + name_off, SDT_NOTE_NAME,
@@ -282,22 +242,14 @@ static size_t dso_sdt_notes(struct dso_symbol **symbols,
Elf *elf)
if (!vaddr)
continue;
+ actor(name, vaddr, arg);
++symbols_count;
- *symbols = realloc_symbols(*symbols, symbols_count);
- if (!*symbols) {
- symbols_count = 0;
- break;
- }
-
- symbol.name = strdup(name);
- symbol.addr = vaddr;
- memcpy(&(*symbols)[symbols_count - 1], &symbol, sizeof(symbol));
}
return symbols_count;
}
-size_t get_dso_symbols(struct dso_symbol **symbols, const char *exec, int type)
+size_t read_dso_symbols(const char *exec, int type, symbol_actor actor, void
*arg)
{
size_t symbols_count = 0;
@@ -315,10 +267,10 @@ size_t get_dso_symbols(struct dso_symbol **symbols, const
char *exec, int type)
if (elf) {
switch (type) {
case FIND_SYMBOL:
- symbols_count = dso_symbols(symbols, elf);
+ symbols_count = dso_symbols(elf, actor, arg);
break;
case FIND_STAPSDT_NOTE:
- symbols_count = dso_sdt_notes(symbols, elf);
+ symbols_count = dso_sdt_notes(elf, actor, arg);
break;
}
diff --git a/userspace/symbol.h b/userspace/symbol.h
index e4ac8a7..a81a39c 100644
--- a/userspace/symbol.h
+++ b/userspace/symbol.h
@@ -23,29 +23,25 @@
#define FIND_SYMBOL 1
#define FIND_STAPSDT_NOTE 2
-struct dso_symbol;
-
#ifndef NO_LIBELF
#include <gelf.h>
#include <sys/queue.h>
typedef GElf_Addr vaddr_t;
+typedef void (* symbol_actor)(const char *name, vaddr_t addr, void *arg);
-struct dso_symbol
-{
- char *name;
- vaddr_t addr;
-};
-
-/**
- * free() symbols and it's names
- */
-void free_dso_symbols(struct dso_symbol *symbols, size_t symbols_count);
/**
- * @return all symbols and notes in DSO.
- * (You must call free_dso_symbols() at end.)
+ * Read all DSO symbols/sdt notes and all for every of them
+ * an actor.
+ *
+ * @exec - path to DSO
+ * @type - see FIND_*
+ * @symbol_actor - actor to call (callback)
+ * @arg - argument for @actor
+ *
+ * @return number of dso symbols found
*/
-size_t get_dso_symbols(struct dso_symbol **symbols, const char *exec, int
type);
+size_t read_dso_symbols(const char *exec, int type, symbol_actor actor, void
*arg);
#endif
--
1.8.4.rc3
Other related posts:
- » [ktap] [PATCH 5/5] symbol: use actor instead of dynamically allocate array of symbols. - Azat Khuzhin
