libsupc++ nit picks

[Nathan Myers]

On Thu, Jan 03, 2002 at 03:17:12PM -0800, Kenny Simpson wrote:
>   In libsupc++ I see the idiom:
> 	if (ptr)
> 	  free (ptr)
> used in a few places ( del_op.cc:38, del_opnt.cc:38,
> eh_globals.cc:51 )
> , but from the UNIX man pages:
> http://www.opengroup.org/cgi-bin/dbcgix?TPL=sud2/xsh/free==FAQBD=1==1
> and Linux man pages
> it is stated that free(0) does nothing, thus making
> this
> 'if' redundant.
> I also found the C rational here:
> http://www.lysator.liu.se/c/rat/d10.html#4-10-3-2

Cases of "if (p) free(p)" do represent mere paranoia.  Patches
to clean them up should be welcome.
 
> Also, the use of qualified names, usings, and extern
> vs #include seem inconsistent.
> To use the above example again, del_op.cc and
> del_opnt.cc refer to ::free, where eh_globals.cc uses
> std::free.

Standard names used within standard implementations should always 
be qualified with "std::", either explicitly or through a 
using-declaration in an enclosing (not global!) scope.  Without
a qualification, you may risk ambiguity with or (worse) diversion 
to another function of the same name in a user's namespace, through
Koenig lookup.

In general, qualifying with std:: is clearer than relying on a
using-declaration elsewhere in the file.

The use of ::free mentioned may be an implementation trick to link 
to the C library's free() without risk of getting the C++ library's 
version.  (I haven't looked.)  Otherwise, it should be std::free.

> del_op.cc uses an extern to find free, where
> eh_globals.cc #include <cstdlib>

Declaring standard components directly, rather than including
the defining header, is always wrong in user code.

Within the implementation the question is muddier; in general it's 
bad engineering to duplicate definitions, because it adds fragility.
Implementations do have some latitude in how they declare names, and 
it should not be necessary to hunt down all the declarations to 
change it.
 
> new_handle.cc uses std:: for some names
> (std::nothrow_t), but usings for others
> (std::new_handler).
> 
> Is this just a result of many different authors?
> Should this part of the library fall under the same
> coding rules as the rest?

It is probably a result of different authors, and of lack of
clear guidance.  Patches welcome.

Nathan Myers
ncm at cantrip dot org