Probe registration fixes #632
Conversation
Eventually all string will be unicode, why not rather enforcing unicode. If you want the api to be forgiving, decorate the api to properly convert from str to unicode if needs be. I seems safe to assume that the unicode strings will only be made of ascii char and will safely convert to the c binding variable using |
| desc = "-:kprobes/%s" % k | ||
| lib.bpf_detach_kprobe(desc.encode("ascii")) | ||
| self._del_kprobe(k) | ||
| for k, v in self.open_uprobes.items(): | ||
| lib.perf_reader_free(v) | ||
| for k in list(self.open_uprobes): |
There was a problem hiding this comment.
Ah, good, I had noticed this one as well, and my fix would have been the same.
There was a problem hiding this comment.
Actually, you could also have done:
for k, v in list(self.open_uprobes.items()):
Which would have avoided the [k] below.
There was a problem hiding this comment.
Ah. I didn't realise that was possible. I can make that change.
* rework `_get_kprobe_functions` to avoid unclosed blacklist warning * rework `cleanup` to avoid changing size of dict while iterating * make handling return of `bpf_function_name` work in py2 and py3
`open_kprobes` is a dict of open kprobes. Its keys are strings for normal probes and a tuple for perf buffers. Normal probes need unregistering on script exit; perf buffers do not. `cleanup` currently looks for string keys (specifically type `str`) when working out what to unregister, which is a bit brittle -- in Python2 strings can be both native `str` and `unicode`, depending what exactly was passed to `attach-*/detach_*` and whether `from __future__ import unicode_literals` is used (e.g. #623). This diff makes the API more relaxed by casting the probe name to `str` to match the expectations of `cleanup`. This works in py2 (with and without unicode_literals) and py3.
The first diff fixes the py3 incompatibilities @drzaeus77 pointed out in 8fa8c0e. In testing (which I did using Ubuntu's alternatives support) I found a couple of other things that needed fixing too.
The second removes the asserts that force
strnames for kprobes (#623). I'm on the fence on this one. On one hand I think it's better to make sure all the probes have string keys, particularly as we use the type to decide which probes to remove, but on the other hand it makes the API unforgiving if you use py2's unicode_literals, or you try to register a probe with a unicode string you already had to hand. @drzaeus77 any opinion here?