This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug tapsets/20363] syscall return probes should provide a richer retstr


https://sourceware.org/bugzilla/show_bug.cgi?id=20363

--- Comment #2 from Josh Stone <jistone at redhat dot com> ---
(In reply to David Smith from comment #1)
> Technically this wouldn't be hard to do. The hardest part would be to decide
> on a format for 'detailed_retstr' that works for all the syscalls. We've got
> syscalls like:
> 
> - pipe(), the 2 file descriptors would be in detailed_retstr
> - read(), the buffer of read characters would be in detailed_retstr
> - ptrace(), that's fairly complicated to decode for detailed_retstr
> 
> I'm sure there are others to think about.
> 
> Perhaps the format would look something like "RETSTR (PARAM1=VALUE,
> PARAM2=VALUE, ...):
> 
> - pipe(): "0 (filedes[0]=3, filedes[1]=4)"

Bikeshedding, I'd probably write "filedes=[3,4]".

This one is already weird because the entry probe is currently showing these
values, when they haven't been set yet!

> - read(): "11 (buf="a1234567890")"
> - ptrace(): each ptrace request would need a different format, something
> like:
>     "123 (request=PEEKTEXT)", "0 (request=PTRACE_GETREGS, data=???)" (I'm
> unsure
>     here how we'd display the registers)

Yeah, GETREGS is tough -- might be ok to skip that one.  But ptrace.return
already defines geteventmsg_data and arch_prctl_addr, so those would be good to
show.

syscall.wait4 is another good candidate with its status_str, although it sets
"N/A" or "NULL" in some cases.  Perhaps the detailed_retstr can be smarter
about omitting data that's not applicable.

> Would you think you'd like the 'retstr' value included in 'detailed_retstr'
> as I've shown here or not? Any other thoughts on the format?
> 
> One more question to think about. Let's talk about a syscall that doesn't
> need any extra info in 'detailed_retstr', like 'write()'. Would write()'s
> detailed retstr look like: '0 ()' or just '0'?

I think just "0".  And for that reason, I think retstr should already be
bundled in detailed_retstr, so script authors don't have to repeat the logic
figuring out whether there's any extra data to show.

But one could also argue that having conditional details would make it harder
for downstream consumers to parse.  Painting bikesheds is hard...

-- 
You are receiving this mail because:
You are the assignee for the bug.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]