Rishi Srivatsavai wrote: > [email protected] wrote:> >> - Exported (via mapfile-vers) functions should first validate that the >> pointerst that they are passed are non-null before following them >> e.g., dladm_valid_bridgename should first check if bridge == NULL. >> Similarly >> the following functions need to validate that the pointer they get >> is non-null before using it: >> dladm_observe_to_bridge, dladm_bridge_enable, dladm_bridge_state . >> dladm_bridge_door_call, dladm_bridge_setlink, dladm_bridge_get_fwdtable, >> dladm_bridge_get_trillnick, dladm_bridge_get_nick, dladm_bridge_set_nick, >> dladm_bridge_get_port_cfg (there may be some that I missed, please check) > > REJECT, as long as the caller understands that the params must be > non-NULL I think such checks will clutter the code. We already make > assumptions on valid dld handle, linkid etc.
I strongly agree with Rishi here, and I'll go further than that: it's impossible to validate that the user passes in the "correct" pointer in all cases, and NULL is just one of a zillion different incorrect pointers. (And likely not even the most interesting one, as referring to a freed structure is a common mistake, and far more difficult to catch.) Moreover, the CPU and kernel give us pointer assertions for free with every dereference. So, if someone did pass in NULL, and if NULL is invalid in that context, we'd drop core, and have a nice core file for use with mdb. That takes zero added code, and works reliably. And on top of that, the result of returning an error code is usually worse. If the caller can't scrape together enough clue to pass in a valid pointer, what's the chance that the caller is also checking error returns diligently? If the caller does check for errors, how can he usefully tell anyone that there's something wrong? syslog(3C) doesn't send email to support. If he passes the error along upwards to someone else who reports the problem, will we still be able to figure out which call deep in the bowels of the system originally failed? For all of those reasons (and perhaps a few more), random checks for NULL at API boundaries has _never_ been the common style in Solaris. That's why the string(3C) functions will drop core if passed NULL. (The recent change to printf(3C) notwithstanding.) I'd be opposed even if this weren't a private library. -- James Carlson 42.703N 71.076W <[email protected]> _______________________________________________ networking-discuss mailing list [email protected]
