Tapset dereferencing audit

Tapset dereferencing audit

[Stone, Joshua I]

Hi all,

I just made a rather large checkin on the tapsets, so I figured it was
worth an announcment.  My goal in reviewing the tapsets was to make sure
that everywhere we dereference potentially-invalid pointers, we use the
appropriate macros to protect against faults.

We've had deref(), store_deref(), and deref_string() for a while for
this purpose, and recently I added kread() and kwrite() for more
convenience (they infer the typecast and size from the pointer type).

I have a few observations:

* Tapset functions written in embedded-C should *not* use a return
statement.  Yes, it works today if you do, but we might not guarantee
that forever.  Don't do it.  If you really need a way to end execution
of the function, use a goto to the end of your function body.

* Some dereferences are still left unchecked.  I left these in places
that make function calls, or use complicated macros.  We'll either need
to manually reproduce the functionality of those calls with our
protection in place, or use some other method to catch any potential
faults.  I marked these with FIXME wherever I saw a potential problem.

* In many places the tapset functions were checking for NULL pointers
and returning a predetermined value, with no error.  I tried to preserve
this as much as possible.  However, I suspect that many of those
NULL-checks were just a feeble attempt to avoid bad pointers -- it's
important to realize that non-NULL might still be bad!

So, my call to the various tapset owners: please review whether each of
your NULL-checks is necessary in light of my new changes.  There are
probably some legitimate cases where a pointer might be NULL but
shouldn't signal an error.  In all other cases, just remove the check
and let kread() throw an error to the user.

As far as the testsuite can show, I didn't introduce any new
regressions.  But of course, everyone please run tests on your own
machines so we can make sure.

Thanks,

Josh