]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Drop unused cbdata.h definitions and re-document
authorAmos Jeffries <squid3@treenet.co.nz>
Wed, 4 Feb 2015 21:37:28 +0000 (13:37 -0800)
committerAmos Jeffries <squid3@treenet.co.nz>
Wed, 4 Feb 2015 21:37:28 +0000 (13:37 -0800)
Remove the now unused cbdataFree, cbdataAlloc, CBDATA_TYPE,
CBDATA_INIT_TYPE, CBDATA_INIT_TYPE_FREECB symbols.

Re-write CBDATA documentation to reflect the current available
API symbols, their usage, and mechanisms that should be used
instead of CBDATA such as AsyncJob/Call and RefCount.

Along with some doxygen polishing to meet currently agreed
style for how to document major code features.

Make generic_cbdata::data a private member. The constructor
and unwrap() operator provide all necessary public API.

Replace store_client.cc use of cbdataInternalLock/Unlock with
a CbcPointer<> smart pointer equivalent. The use remains an
abuse of CBDATA, just no longer directly referencing the
internal API functions.

src/cbdata.cc
src/cbdata.h
src/store_client.cc

index 6c46a5de64588c901b7f1f69c5ba277a0b0c0689..c125eb03f0a8cdb82d35ec3f10a38fa6601c7992 100644 (file)
@@ -8,19 +8,6 @@
 
 /* DEBUG: section 45    Callback Data Registry */
 
-/**
- \defgroup CBDATAInternal Callback Data Allocator Internals
- \ingroup CBDATAAPI
- *
- * These routines manage a set of registered callback data pointers.
- * One of the easiest ways to make Squid coredump is to issue a
- * callback to for some data structure which has previously been
- * freed.  With these routines, we register (add) callback data
- * pointers, lock them just before registering the callback function,
- * validate them before issuing the callback, and then free them
- * when finished.
- */
-
 #include "squid.h"
 #include "cbdata.h"
 #include "Generic.h"
@@ -58,10 +45,17 @@ public:
 
 #endif
 
-/// \ingroup CBDATAInternal
 #define OFFSET_OF(TYPE, MEMBER) ((size_t) &(((TYPE) *)0)->(MEMBER))
 
-/// \ingroup CBDATAInternal
+/**
+ * Manage a set of registered callback data pointers.
+ * One of the easiest ways to make Squid coredump is to issue a
+ * callback to for some data structure which has previously been
+ * freed.  With this class, we register (add) callback data
+ * pointers, lock them just before registering the callback function,
+ * validate them before issuing the callback, and then free them
+ * when finished.
+ */
 class cbdata
 {
 #if !HASHED_CBDATA
@@ -140,14 +134,12 @@ static OBJH cbdataDump;
 static OBJH cbdataDumpHistory;
 #endif
 
-/// \ingroup CBDATAInternal
 struct CBDataIndex {
     MemAllocator *pool;
     FREE *free_func;
 }
 *cbdata_index = NULL;
 
-/// \ingroup CBDATAInternal
 int cbdata_types = 0;
 
 #if HASHED_CBDATA
index cf4c4a48af022085b259dfd0ee3c2d04e708d8de..c28c1c4b3cab0a2e8c2bb92755b153ee764c3262 100644 (file)
@@ -6,41 +6,45 @@
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
-#ifndef   SQUID_CBDATA_H
-#define   SQUID_CBDATA_H
+#ifndef SQUID_SRC_CBDATA_H
+#define SQUID_SRC_CBDATA_H
 
 #include "typedefs.h"
 
 /**
- \defgroup CBDATAAPI Callback Data Allocator API
- \ingroup Components
+\page CBDATA Callback Data Allocator API
+
+ \section  Introduction
+
  \par
- *    Squid's extensive use of callback functions makes it very
- *    susceptible to memory access errors. To address this all callback
- *    functions make use of a construct called cbdata. This allows
- *    functions doing callbacks to verify that the caller is still
- *    valid before making the callback.
- *
- \note  cbdata is intended for callback data and is tailored specifically
- *      to make callbacks less dangerous leaving as few windows of errors as
- *      possible. It is not suitable or intended as a generic RefCount
- *      memory allocator.
- *
- *
+   Squid's extensive use of callback functions makes it very
+   susceptible to memory access errors. To address this all callback
+   functions make use of a construct called cbdata. This allows
+   functions doing callbacks to verify that the caller is still
+   valid before making the callback.
+
+ \note cbdata is intended for callback data and is tailored specifically
+       to make callbacks less dangerous leaving as few windows of errors as
+       possible. It is not suitable or intended as a generic RefCount
+       memory allocator.
+
+ \par
+   The AsyncJob/AsyncCall mechanism is preferred over CBDATA.
+   It replaces cbdata with an AsyncCall::Pointer object which
+   performs the same memory protection duties via other means.
+
  \section Examples Examples
  \par
*  Here you can find some examples on how to use cbdata, and why.
- *
+   Here you can find some examples on how to use cbdata, and why.
+
  \subsection AsyncOpWithoutCBDATA Asynchronous operation without cbdata, showing why cbdata is needed
  \par
*  For a asyncronous operation with callback functions, the normal
*  sequence of events in programs NOT using cbdata is as follows:
- *
+   For a asyncronous operation with callback functions, the normal
+   sequence of events in programs NOT using cbdata is as follows:
+
  \code
     // initialization
-    type_of_data our_data;
-    ...
-    our_data = malloc(...);
+    type_of_data our_data = new ...;
     ...
     // Initiate a asyncronous operation, with our_data as callback_data
     fooOperationStart(bar, callback_func, our_data);
     // The asyncronous operation completes and makes the callback
     callback_func(callback_data, ....);
     // Some time later we clean up our data
-    free(our_data);
+    delete our_data;
  \endcode
- *
+
  \par
*  However, things become more interesting if we want or need
*  to free the callback_data, or otherwise cancel the callback,
*  before the operation completes. In constructs like this you
*  can quite easily end up with having the memory referenced
*  pointed to by callback_data freed before the callback is invoked
*  causing a program failure or memory corruption:
- *
+   However, things become more interesting if we want or need
+   to free the callback_data, or otherwise cancel the callback,
+   before the operation completes. In constructs like this you
+   can quite easily end up with having the memory referenced
+   pointed to by callback_data freed before the callback is invoked
+   causing a program failure or memory corruption:
+
  \code
     // initialization
-    type_of_data our_data;
-    ...
-    our_data = malloc(...);
+    type_of_data our_data = new ...;
     ...
     // Initiate a asyncronous operation, with our_data as callback_data
     fooOperationStart(bar, callback_func, our_data);
     ...
     // ouch, something bad happened elsewhere.. try to cleanup
     // but the programmer forgot there is a callback pending from
-    // fooOperationsStart() (an easy thing to forget when writing code
+    // fooOperationsStart(). An easy thing to forget when writing code
     // to deal with errors, especially if there may be many different
-    // pending operation)
-    free(our_data);
+    // pending operations.
+    delete our_data;
     ...
     // The asyncronous operation completes and makes the callback
     callback_func(callback_data, ....);
     // CRASH, the memory pointer to by callback_data is no longer valid
     // at the time of the callback
  \endcode
- *
+
  \subsection AsyncOpWithCBDATA Asyncronous operation with cbdata
- *
+
  \par
*  The callback data allocator lets us do this in a uniform and
*  safe manner.  The callback data allocator is used to allocate,
*  track and free memory pool objects used during callback
*  operations.  Allocated memory is locked while the asyncronous
*  operation executes elsewhere, and is freed when the operation
*  completes.  The normal sequence of events is:
- *
+   The callback data allocator lets us do this in a uniform and
+   safe manner.  The callback data allocator is used to allocate,
+   track and free memory pool objects used during callback
+   operations.  Allocated memory is locked while the asyncronous
+   operation executes elsewhere, and is freed when the operation
+   completes.  The normal sequence of events is:
+
  \code
     // initialization
-    type_of_data our_data;
-    ...
-    our_data = cbdataAlloc(type_of_data);
+    type_of_data our_data = new type_of_data;
     ...
     // Initiate a asyncronous operation, with our_data as callback_data
     fooOperationStart(..., callback_func, our_data);
     void *cbdata;
     if (cbdataReferenceValidDone(local_pointer, &amp;cbdata))
         callback_func(...., cbdata);
-    ...
-    cbdataFree(our_data);
+    delete our_data;
  \endcode
- *
+
  \subsection AsynchronousOpCancelledByCBDATA Asynchronous operation cancelled by cbdata
- *
+
  \par
*  With this scheme, nothing bad happens if cbdataFree() gets called
*  before fooOperantionComplete(...).
- *
  With this scheme, nothing bad happens if delete gets called
+   before fooOperantionComplete(...).
+
  \par   Initalization
  \code
-    type_of_data our_data;
+    // initialization
+    type_of_data our_data = new type_of_data;
     ...
-    our_data = cbdataAlloc(type_of_data);
- \endcode
- *  Initiate a asyncronous operation, with our_data as callback_data
- \code
+    // Initiate a asyncronous operation, with our_data as callback_data
     fooOperationStart(..., callback_func, our_data);
- \endcode
- *  do some stuff with it
- \code
+    ...
+    // do some stuff with it
     void *local_pointer = cbdataReference(callback_data);
- \endcode
- *  something bad happened elsewhere.. cleanup
- \code
-    cbdataFree(our_data);
- \endcode
- *  The asyncronous operation completes and tries to make the callback
- \code
+    ...
+    // something bad happened elsewhere.. cleanup
+    delete our_data;
+    ....
+    // The asyncronous operation completes and makes the callback
     void *cbdata;
     if (cbdataReferenceValidDone(local_pointer, &amp;cbdata))
-        {
- \endcode
- *  won't be called, as the data is no longer valid
- \code
+        // won't be called, as the data is no longer valid
         callback_func(...., cbdata);
-    }
+    delete our_data;
  \endcode
- *
+
  \par
- *  In this case, when cbdataFree() is called before
*  cbdataReferenceValidDone(), the callback_data gets marked as invalid.
*  When the callback_data is invalid before executing the callback
*  function, cbdataReferenceValidDone() will return 0 and
*  callback_func is never executed.
- *
+   In this case, when delete is called before cbdataReferenceValidDone(),
  the callback_data gets marked as invalid.
+   When the callback_data is invalid before executing the callback
+   function, cbdataReferenceValidDone() will return 0 and
+   callback_func is never executed.
+
  \subsection AddingCBDATAType Adding a new cbdata registered type
- *
+
+ \par
+   To add new module specific data types to the allocator one uses
+   the macro CBDATA_CLASS() in the class private section, and
+   CBDATA_CLASS_INIT() or CBDATA_NAMESPACED_CLASS_INIT() in the
+   class .cc file.
+
+ \code
+    class Foo
+    {
+        CBDATA_CLASS(Foo);
+
+    public:
+        Foo() {}
+        ~Foo() {}
+    };
+    ...
+    CBDATA_CLASS_INIT(Foo);
+ \endcode
+
+ \par
+   These macros create new(), delete() and toCbdata() methods
+   definition in class scope. Any allocate calls must be made with
+   new() and destruction with delete(), they may be called from
+   anywhere.
+
+ \par
+   The class constructor must make sure that all member
+   variables are initialized, and the class destructor that all
+   dynamic memory is released.
+
  \par
- *  To add new module specific data types to the allocator one uses the
- *  macro CBDATA_CLASS() in the class private section, and CBDATA_CLASS_INIT()
- *      or CBDATA_NAMESPACED_CLASS_INIT() in the .cc file.
- *      This creates new(), delete() and toCbdata() methods
- *  definition in class scope. Any allocate calls must be made with
- *      new() and destruction with delete(), they may be called from anywhere.
+   The CbcPointer<> template should be used to create a smart-pointer
+   type for simple reference tracking. It provides get() and valid()
+   accessors for use instead of cbdataReferenceValid(), and performs
+   reliable automatic cbdataReference() and cbdataReferenceDone()
+   tracking.
+   Note that it does NOT provide a replacement for cbdataReferenceValidDone().
+
  */
 
 /**
- *\ingroup CBDATAAPI
  * cbdata types. Similar to the MEM_* types, but managed in cbdata.cc
  * A big difference is that cbdata types are dynamically allocated.
- * Initially only UNKNOWN type is predefined. Other types are added runtime.
+ *
+ * Initially only UNKNOWN type is predefined.
+ * Other types are added at runtime by CBDATA_CLASS().
  */
 typedef int cbdata_type;
 static const cbdata_type CBDATA_UNKNOWN = 0;
 
-/// \ingroup CBDATAAPI
+/**
+ * Create a run-time registration of CBDATA component with
+ * the Squid cachemgr
+ */
 void cbdataRegisterWithCacheManager(void);
 
 /**
  * Allocates a new entry of a registered CBDATA type.
- * \deprecated use CBDATA_CLASS() instead
+ *
+ * \note For internal CBDATA use only.
  */
 void *cbdataInternalAlloc(cbdata_type type, const char *, int);
-/// \deprecated use CBDATA_CLASS() instead
-#define cbdataAlloc(type) ((type *)cbdataInternalAlloc(CBDATA_##type,__FILE__,__LINE__))
 
 /**
- * Frees a entry allocated by cbdataAlloc().
+ * Frees a entry allocated by cbdataInternalAlloc().
+ *
+ * Once this has been called cbdataReferenceValid() and
+ * cbdataReferenceValidDone() will return false regardless
+ * of whether there are remaining cbdata references.
  *
- \note  If there are active references to the entry then the entry
- *      will be freed with the last reference is removed. However,
- *      cbdataReferenceValid() will return false for those references.
- * \deprecated use CBDATA_CLASS() instead
+ * cbdataReferenceDone() must still be called for any active
+ * references to the cbdata entry. The cbdata entry will be freed
+ * only when the last reference is removed.
+ *
+ * \note For internal CBDATA use only.
  */
 void *cbdataInternalFree(void *p, const char *, int);
-/// \deprecated use CBDATA_CLASS() instead
-#define cbdataFree(var)     do {if (var) {cbdataInternalFree(var,__FILE__,__LINE__); var = NULL;}} while(0)
 
 #if USE_CBDATA_DEBUG
 void cbdataInternalLockDbg(const void *p, const char *, int);
-#define cbdataInternalLock(a)       cbdataInternalLockDbg(a,__FILE__,__LINE__)
+#define cbdataInternalLock(a) cbdataInternalLockDbg(a,__FILE__,__LINE__)
 
 void cbdataInternalUnlockDbg(const void *p, const char *, int);
-#define cbdataInternalUnlock(a)     cbdataInternalUnlockDbg(a,__FILE__,__LINE__)
+#define cbdataInternalUnlock(a) cbdataInternalUnlockDbg(a,__FILE__,__LINE__)
 
 int cbdataInternalReferenceDoneValidDbg(void **p, void **tp, const char *, int);
 #define cbdataReferenceValidDone(var, ptr) cbdataInternalReferenceDoneValidDbg((void **)&(var), (ptr), __FILE__,__LINE__)
@@ -223,8 +251,8 @@ void cbdataInternalUnlock(const void *p);
             callback(..., cbdata);
  \endcode
  *
- \param var The reference variable. Will be automatically cleared to NULL.
- \param ptr A temporary pointer to the referenced data (if valid).
\param var The reference variable. Will be automatically cleared to NULL.
\param ptr A temporary pointer to the referenced data (if valid).
  */
 int cbdataInternalReferenceDoneValid(void **p, void **tp);
 #define cbdataReferenceValidDone(var, ptr) cbdataInternalReferenceDoneValid((void **)&(var), (ptr))
@@ -234,109 +262,91 @@ int cbdataInternalReferenceDoneValid(void **p, void **tp);
 /**
  * \param p A cbdata entry reference pointer.
  *
- * \retval 0    A reference is stale. The pointer refers to a entry freed by cbdataFree().
+ * \retval 0    A reference is stale. The pointer refers to a entry already freed.
  * \retval true The reference is valid and active.
  */
 int cbdataReferenceValid(const void *p);
 
-/// \ingroup CBDATAAPI
+/**
+ * Create a run-time registration for the class type with cbdata memory allocator.
+ *
+ * \note For internal CBDATA use only.
+ */
 cbdata_type cbdataInternalAddType(cbdata_type type, const char *label, int size, FREE * free_func);
 
 /**
  * This needs to be defined FIRST in the class definition.
  * It plays with private/public states in C++.
  */
-#define CBDATA_CLASS(type)  \
+#define CBDATA_CLASS(type) \
     public: \
         void *operator new(size_t size) { \
           assert(size == sizeof(type)); \
-          if (!CBDATA_##type) \
-                      CBDATA_##type = cbdataInternalAddType(CBDATA_##type, #type, sizeof(type), NULL); \
+          if (!CBDATA_##type) CBDATA_##type = cbdataInternalAddType(CBDATA_##type, #type, sizeof(type), NULL); \
           return (type *)cbdataInternalAlloc(CBDATA_##type,__FILE__,__LINE__); \
         } \
         void operator delete (void *address) { \
-          if (address) cbdataInternalFree(address,__FILE__,__LINE__);\
+          if (address) cbdataInternalFree(address,__FILE__,__LINE__); \
         } \
-                void *toCbdata() { return this; } \
+        void *toCbdata() { return this; } \
     private: \
-    static cbdata_type CBDATA_##type;
+       static cbdata_type CBDATA_##type;
 
 /**
- \par
- *    Creates a new reference to a cbdata entry. Used when you need to
- *    store a reference in another structure. The reference can later
- *    be verified for validity by cbdataReferenceValid().
+ * Creates a global instance pointer for the CBDATA memory allocator
+ * to allocate and free objects for the matching CBDATA_CLASS().
  *
- \param var
- *       The reference variable is a pointer to the entry, in all
- *       aspects identical to the original pointer. But semantically it
- *       is quite different. It is best if the reference is thought of
- *       and handled as a "void *".
- */
-#define cbdataReference(var)    (cbdataInternalLock(var), var)
-
-/**
- \ingroup CBDATAAPI
- * Removes a reference created by cbdataReference().
+ * Place this in the appropriate .cc file for the class being registered.
  *
- \param var The reference variable. Will be automatically cleared to NULL.
+ * May be placed inside an explicit namespace scope declaration,
+ * or CBDATA_NAMESPACED_CLASS_INIT() used instead.
  */
-#define cbdataReferenceDone(var) do {if (var) {cbdataInternalUnlock(var); var = NULL;}} while(0)
-
-/// \ingroup CBDATAAPI
 #define CBDATA_CLASS_INIT(type) cbdata_type type::CBDATA_##type = CBDATA_UNKNOWN
-#define CBDATA_NAMESPACED_CLASS_INIT(namespace, type) cbdata_type namespace::type::CBDATA_##type = CBDATA_UNKNOWN
 
 /**
- * Macro that defines a new cbdata datatype. Similar to a variable
- * or struct definition. Scope is always local to the file/block
- * where it is defined and all calls to cbdataAlloc() for this type
- * must be within the same scope as the CBDATA_TYPE declaration.
- * Allocated entries may be referenced or freed anywhere with no
- * restrictions on scope.
- * \deprecated Use CBDATA_CLASS() instead
+ * Creates a global instance pointer for the CBDATA memory allocator
+ * to allocate and free objects for the matching CBDATA_CLASS().
+ *
+ * Place this in the appropriate .cc file for the class being registered.
  */
-#define CBDATA_TYPE(type)   static cbdata_type CBDATA_##type = CBDATA_UNKNOWN
+#define CBDATA_NAMESPACED_CLASS_INIT(namespace, type) cbdata_type namespace::type::CBDATA_##type = CBDATA_UNKNOWN
 
 /**
- \ingroup CBDATAAPI
+ * Creates a new reference to a cbdata entry. Used when you need to
+ * store a reference in another structure. The reference can later
+ * be verified for validity by cbdataReferenceValid().
  *
- * Initializes the cbdatatype. Must be called prior to the first use of cbdataAlloc() for the type.
+ * \deprecated Prefer the use of CbcPointer<> smart pointer.
  *
\par
- * Alternative to CBDATA_INIT_TYPE()
- *
- \param type        Type being initialized
\param free_func   The freehandler called when the last known reference to an allocated entry goes away.
* \param var
+ *       The reference variable is a pointer to the entry, in all
+ *       aspects identical to the original pointer. But semantically it
+ *       is quite different. It is best if the reference is thought of
*       and handled as a "void *".
  */
-#define CBDATA_INIT_TYPE_FREECB(type, free_func) do { if (!CBDATA_##type) CBDATA_##type = cbdataInternalAddType(CBDATA_##type, #type, sizeof(type), free_func); } while (false)
+#define cbdataReference(var)    (cbdataInternalLock(var), var)
 
 /**
- * Initializes the cbdatatype. Must be called prior to the first use of cbdataAlloc() for the type.
- *
- \par
- * Alternative to CBDATA_INIT_TYPE_FREECB()
+ * Removes a reference created by cbdataReference().
  *
- \param type        Type being initialized
+ * \deprecated Prefer the use of CbcPointer<> smart pointer.
  *
- * \deprecated Use CBDATA_CLASS() instead
+ * \param var The reference variable. Will be automatically cleared to NULL.
  */
-#define CBDATA_INIT_TYPE(type)  CBDATA_INIT_TYPE_FREECB(type, NULL)
+#define cbdataReferenceDone(var) do {if (var) {cbdataInternalUnlock(var); var = NULL;}} while(0)
 
 /**
- \ingroup CBDATA
- *
- * A generic wrapper for passing objects through cbdata.
+ * A generic wrapper for passing object pointers through cbdata.
  * Use this when you need to pass callback data to a blocking
- * operation, but you don't want to/cannot have that pointer be cbdata itself.
+ * operation, but you don't want to/cannot have that pointer be
+ * cbdata itself.
  */
 class generic_cbdata
 {
     CBDATA_CLASS(generic_cbdata);
 
 public:
-
-    generic_cbdata(void * aData) : data(aData) {}
+    generic_cbdata(void *aData) : data(aData) {}
 
     template<typename wrapped_type>void unwrap(wrapped_type **output) {
         *output = static_cast<wrapped_type *>(data);
@@ -344,14 +354,7 @@ public:
     }
 
 private:
-    /**
-     * The wrapped data - only public to allow the mild abuse of this facility
-     * done by store_swapout - it gives a wrapped StoreEntry to StoreIO as the
-     * object to be given to the callbacks. That needs to be fully cleaned up!
-     * - RBC 20060820
-     \todo CODE: make this a private field.
-     */
-    void *data; /* the wrapped data */
+    void *data;
 };
 
 #endif /* SQUID_CBDATA_H */
index 6bff6689db9e372e07fbe9fe86d76de12c2320e3..fd677f2887a63632779247f071c96a58d174310d 100644 (file)
@@ -308,13 +308,11 @@ storeClientCopy2(StoreEntry * e, store_client * sc)
      * this function
      * XXX: Locking does not prevent calling sc destructor (it only prevents
      * freeing sc memory) so sc may become invalid from C++ p.o.v.
-     *
      */
-    cbdataInternalLock(sc);
+    CbcPointer<store_client> tmpLock = sc;
     assert (!sc->flags.store_copying);
     sc->doCopy(e);
-    assert (!sc->flags.store_copying);
-    cbdataInternalUnlock(sc);
+    assert(!sc->flags.store_copying);
 }
 
 void