Several tapset functions that return strings signal abnormal inputs or conditions by returning values like "BAD VALUE", "<unknown>", "", "NULL", "N/A", "UNKNOWN", "far far in the future...", and my personal favorite, "a long, long time ago...". It would be better if, for a future --compatible=VERSION level, these tapset functions returned errors instead, getting the caller scripts to use the try/catch construct to handle errors.
I've been working on this for about a week now. Although most of the tapset has been checked for error return strings, there are a two locations where they were left alone: - tapset/linux/aux_syscalls.stp: many (all?) of the functions which take in an address will return "NULL" if the address is 0, or "UNKNOWN" if _stp_copy_from_user() fails. These functions are used for the most part in probe aliases in tapset/linux/*syscalls*.stp files, e.g. in the argstr variable. Thus, it provides 'best effort' values to the user. A shift towards assuring 'correct' values would require a mechanism that allows users to be notified of the failure in probe aliases. - tapset/linux/vfs.stp: the same discussion applies here, except that the function in question is __find_bdevname(), which is used in many probe aliases in the same file. __find_bdevname() returns "N/A" if bdev is 0 and otherwise calls bdevname().
I pushed my changes so far. Currently running the testsuite with v2.3 hardcoded in source code to see where I need to change it. Will try to update & push the testsuite tomorrow with 2.3 hardcoded.
Finished updating the testsuite to accomodate the changes. The testsuite was changed in the following places: - systemtap.base/vma_vdso.stp: wrapped umodname() call in try/catch block - systemtap.stress/conversions.stp: added two tests for the two new functions in the uconversions.stp tapset and wrapped user_string_n() in try/catch block - systemtap.stress/conversions.exp: changed expected errors and warnings from 18 to 20 to account for the two new functions which output warnings Related commits: http://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commit;h=c3af65d7784d36935c02e9dcb621deba97b9d960 http://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commit;h=1c40059d13e69e15a44f1526966bd04d0295ad22 Also note that the following functions had their patches reversed and left alone for the same reasons mentioned in Comment 1: - tapset/linux/ip.stp: format_ipaddr() - tapset/errno.stp: returnstr() - tapset/uconversions.stp: user_string() Related commit: http://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commit;h=a904a1c72a51ae7884031e5f679194ad9cb31d67
I figured I should make a comment here before I start forgetting all the details. After discussions with fche and jistone, I reversed the decision on user_string() and made it (again) throw an error. All the probe aliases which used user_string() now use user_string_quoted(). Also, all occurrences of text_strn() were replaced by user_string_n_quoted() to avoid a double-quoting issue. In the process, many of the functions in conversions.stp were changed so that they simply call a more generic form of themselves. The testsuite was updated to reflect the changes in output, notably the presence of quotes around variables, and no longer outputting NULL, but (null) (not a cutesy, but the result of printf'ing a void pointer). Related commits: http://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commit;h=4d0fa8ced16657fe4ef4bf788d35097cb66586a6 (and the commits that follow)
Closing this PR as the great majority of cutesy errors have been wiped. However, as mentioned in comment 1, there is still a major stronghold of cutesiness residing in aux_syscalls.stp. Their total eradication would require work outside the scope of this PR, thus they will be left alone... for now.