Re: [Patch] Bitmap allocator doygen style comments.

[Dhruv Matani <dhruvbird at gmx dot net>]

Ok, thanks for the tips (and the comprehensive spell-check ;-) )

I'll make the necessary corrections.
Ok, just one thing. How much is the acceptable brief description? I'll
take it to be 1 sentence, but I'll wait for your reply before preparing
the patch.

-Dhruv.


On Thu, 2004-12-16 at 19:25, Jonathan Wakely wrote:
> 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
-- 
        -Dhruv Matani.
http://www.geocities.com/dhruvbird/

The price of freedom is responsibility, but it's a bargain, because
freedom is priceless. ~ Hugh Downs