Re: [Patch] Bitmap allocator doygen style comments.

[Jonathan Wakely <cow at compsoc dot man dot ac dot uk>]

On Thu, Dec 16, 2004 at 06:17:46PM +0530, Dhruv Matani wrote:

> Hello,
> 	This patch adds doxygen style comments for the internal functions.

Cool!
Some corrections and comments below...


>  #if defined __GTHREADS
> -  // _Mutex is an OO-Wrapper for __gthread_mutex_t. It does not allow
> -  // you to copy or assign an already initialized mutex. This is used
> -  // merely as a convenience for the locking classes.
> +  /** @class _Mutex bitmap_allocator.h bitmap_allocator.h
> +   *
> +   * @brief _Mutex is an OO-Wrapper for __gthread_mutex_t. It does not
> +   * allow you to copy or assign an already initialized mutex. This is
> +   * used merely as a convenience for the locking classes.
> +   */

We use a particular style of indenting for our Doxygen comments, so in
all other files I think the above would be:

  /** @class  _Mutex bitmap_allocator.h bitmap_allocator.h
   *
   *  @brief  _Mutex is an OO-Wrapper for __gthread_mutex_t.
   *
   *  It does not allow you to copy or assign an already initialized mutex.
   *  This is used merely as a convenience for the locking classes.
   */

> +  /** @class _Lock bitmap_allocator.h bitmap_allocator.h
> +   *
> +   * @brief _Lock is a simple manual lokcing class which allows you to
                                        ^^^^^^^
                                        locking
> +   * manually lock and unlock a mutex associated with the lock. There
> +   * is not automatic locking or unlocking happening without the
> +   * programmer's explicit instructions. This class unlocks the mutex
> +   * ONLY if it has not been locked. However, this check does not
> +   * apply for lokcing, and wayward use may cause dead-locks.
                  ^^^^^^^
                  locking

> +  /** @class _Auto_Lock bitmap_allocator.h bitmap_allocator.h
> +   *
> +   * @brief _Auto_Lock locks the associated mutex on construction, and
> +   * unlocks on it's destruction. There are no checks performed, and
                   ^^^^
                   its
> +   * this calss follows the RAII principle.
             ^^^^^
             class

> +    /** @class __mini_vector bitmap_allocator.h bitmap_allocator.h
> +     *
> +     * @brief __mini_vector<> is to be used only for built-in types or
> +     * PODs. It is a stripped down version of the full-fledged
> +     * std::vector<>. Noteable differences are: 
                         ^^^^^^^^
                         Notable
> +     * 
> +     * @detail 1. Not all accessor functions are present.
> +     * 2. Used ONLY for PODs.
> +     * 3. No Allocator template argument. Uses ::operator new() to get
> +     * memory, and ::operator delete() to free it.
> +     */

> +    /** @class _Bitmap_counter bitmap_allocator.h bitmap_allocator.h
> +     *
> +     * @brief The bitmap counter which acts as the bitmap manipulator,
> +     * and manages the bit-manipulation functions and the seraching
                                                             ^^^^^^^^^
                                                             searching
> +     * and identification functions on teh bit-map.
                                          ^^^
                                          the
> +     */
>      // _Tp should be a pointer type.
>      template<typename _Tp>
>        class _Bitmap_counter
> @@ -646,11 +676,17 @@ namespace __gnu_cxx
>      }
>    } // namespace balloc
>  
> -  // Generic Version of the bsf instruction.
> +  /* @brief Generic Version of the bsf instruction.
> +   */
>    inline size_t 
>    _Bit_scan_forward(size_t __num)
>    { return static_cast<size_t>(__builtin_ctzl(__num)); }
>  
> +  /** @class free_list bitmap_allocator.h bitmap_allocator.h
> +   *
> +   * @brief The free list class for managing chinks of memory to be
                                                ^^^^^^
                                                chunks
> +   * given to and returned by the bitmap_allocator.
> +   */
>    class free_list
>    {
>      typedef size_t* value_type;
> @@ -669,7 +705,12 @@ namespace __gnu_cxx
>      static _Mutex _S_bfl_mutex;
>  #endif
>      static vector_type _S_free_list;
> -    
> +
> +    /** @brief Validates the memory block passed to this function and
> +     * appropitely performs the action of managing the free list of
          ^^^^^^^^^^^
          appropriately
> +     * blocks by adding this block to the free list or deleting this
> +     * or larger blocks from the free list.
> +     */
>      void
>      _M_validate(size_t* __addr) throw()
>      {
> @@ -704,6 +745,17 @@ namespace __gnu_cxx
>        _S_free_list.insert(__temp, __addr);
>      }
>  
> +    /** @brief Decides whether the wastage of memory is acceptable for
> +     * the current memory request and returns accordingly.
> +     *
> +     * @param __block_size The size of the block available in he free
> +     * list.
> +     *
> +     * @param __required_size The required size of the memory block.
> +     *
> +     * @return true if the wastage incured is acceptable, else returns
                                      ^^^^^^^
                                      incurred
> +     * false.
> +     */
>      bool 
>      _M_should_i_give(size_t __block_size, 
>  		     size_t __required_size) throw()
> @@ -820,12 +872,12 @@ namespace __gnu_cxx
>        }
>  #endif
>  
> -      // Complexity: O(1), but internally depends upon the complexity
> -      // of the function free_list::_M_get. The
> -      // part where the bitmap headers are written is of worst case
> -      // complexity: O(X),where X is the number of blocks of size
> -      // sizeof(value_type) within the newly acquired block. Having a
> -      // tight bound.
> +      /** @brief Complexity: O(1), but internally depends upon the
> +       * complexity of the function free_list::_M_get. The part where
> +       * the bitmap headers are written has complexity: O(X),where X
> +       * is the number of blocks of size sizeof(value_type) within the
> +       * newly acquired block. Having a tight bound.
> +       */

This is very long for a @brief description (as are some of the other
@brief descriptions)

> +      /** @brief Complexity: Worst case complexity is O(N), but that
> +       * is hardly ever hit. if and when this particular case is
                                ^^
                                If
> +       * encountered, the next few cases are guaranteed to have a
> +       * worst case complexity of O(1)!  That's why this function
> +       * performs very well on the average. you can consider this
                                               ^^^
                                               You
> +       * function to be having a complexity referred to commonly as:
                     ^^^^^^^^^^^^
                     to have
> +       * Amortized Constant time.
> +       */
>        pointer 
>        _M_allocate_single_object() throw(std::bad_alloc)
>        {

jon

-- 
Emacs is a nice OS - but it lacks a good text editor.
That's why I am using Vim.
	- Anonymous