Re: RFC: Building temporary char** property arrays

Hi Ben,

I think this is a great idea. The arrays of const char *'s have always
been a pain to construct. I'm all in favour provided:

(a) you show that this system interfaces well with our PieceTable,
(b) that the ownership of the new class is clear
(c) and we have some good example code that clearly shows how/when the
char *'s are deleted (to avoid both memory leaks and unreferenced
pointers).

Thanks very much!

Martin

On Sat, Dec 11, 2010 at 10:51 AM, Ben Martin
<monkeyiq@users.sourceforge.net> wrote:
> Hi,
>  I notice a fair bit in some parts of Abiword that I'm hacking on code
> like the following:
> const gchar* ppAtts[10];
> int i=0;
> ppAtts[i++] = PT_REVISION_ATTRIBUTE_NAME;
> ppAtts[i++] = "foo";
> ppAtts[i++] = 0;
> somethingThatWantsArrayOfCharPointers(ppAtts);
>
> Where ppAtts is a NULL terminated list of pointers to char* which are
> not duplicated/freed. ie, const strings or std::string::c_str() where
> the string object lasts beyond the scope that ppAtts is required for.
>
> I was thinking of replacing some / phasing in the use of boost::array
> for these. The gains being that the allocation of the array on the stack
> is retained, explicit null termination becomes optional, and the "top"
> array index can be maintained by the class for you (avoiding i++ in the
> above). So the code becomes something like this:
>
> propertyArray<> ppAtts;
> ppAtts.push_back( PT_REVISION_ATTRIBUTE_NAME );
> ppAtts.push_back( "foo" );
> // optional
> ppAtts.push_back( 0 );
>
> somethingThatWantsArrayOfCharPointers(ppAtts.data());
>
> As boost::array is a template I have left my prototype as one too, with
> a default size of 32 which should cover the bulk of these temporary
> arrays. The main motivation was to avoid tracking multiple array/index
> pairs as autovars and also in cases where there is one or more if()
> blocks that complicate the flow while building the array. In these cases
> knowing that explicit null termination is not required might avoid
> subtle bugs depending on the code paths executed.
>
> At the moment I zero the array at construction time, but the null
> termination could be added in the data() method to avoid initialization
> overhead. I used delegation to boost::array rather than inheritance to
> allow better control over m_highestUsedIndex and possible lazy null
> termination.
>
> Thoughts welcome... class below...
>
> #include <boost/array.hpp>
>
> /**
>  * As at 2010 there are many pieces of code that declare an array of
>  * char* on the stack to set 2-10 parameters and pass this char** to a
>  * function to use. It is ugly and somewhat error prone to maintain
>  * the index into this array manually and also if code happens to
>  * overrun the array there is no error detection for that. By using
>  * boost::array at least these out of bounds cases can be cought and
>  * code does not have to worry about NULL terminating the array
>  * itself. The array is allocated as part of the object on the stack
>  * though so we avoid memory allocation and zeroing issues along with
>  * possible fragmentation over time. Note that the [i++] style should
>  * also work with propertyArray, and count() gives you the index of
>  * the first NULL value in the array.
>  *
>  * Example usage:
>  * propertyArray<> ppAtts;
>  * ppAtts.push_back( PT_REVISION_ATTRIBUTE_NAME );
>  * ppAtts.push_back( "foo" );
>  * ppAtts.push_back( 0 );    // optional
>  * somethingThatWantsArrayOfCharPointers(ppAtts.data());
>  * and the array goes away here.
>  *
>  * This replaces the style below, note that the code is similar,
>  * though the explicit index "i" is not needed above.
>  *
>  * const gchar* ppAtts[10];
>  * bzero(ppAtts, 10 * sizeof(gchar*));
>  * int i=0;
>  * ppAtts[i++] = PT_REVISION_ATTRIBUTE_NAME;
>  * ppAtts[i++] = "foo";
>  * ppAtts[i++] = 0;
>  * somethingThatWantsArrayOfCharPointers(ppAtts);
>  *
>  */
> template< std::size_t N = 32 >
> class propertyArray
> {
> public:
>    typedef const gchar* T;
>    typedef boost::array< const gchar*, N > m_boostArray_t;
>    typedef typename m_boostArray_t::value_type value_type;
>    typedef typename m_boostArray_t::iterator   iterator;
>    typedef typename m_boostArray_t::const_iterator   const_iterator;
>    typedef typename m_boostArray_t::reference reference;
>    typedef typename m_boostArray_t::const_reference const_reference;
>    typedef std::size_t    size_type;
>
>    propertyArray()
>        :
>        m_highestUsedIndex( 0 )
>    {
>        m_array.assign( 0 );
>    }
>
>    void push_back( const gchar* v )
>    {
>        std::size_t sz = count();
>        m_array.at(sz) = v;
>    }
>
>    std::size_t count() const
>    {
>        return m_highestUsedIndex;
>    }
>
>    const_reference operator[](size_type i) const
>    {
>        return m_array.at(i);
>    }
>    static size_type size() { return N; }
>    T* data() { return m_array.data(); }
>
>
> private:
>    std::size_t     m_highestUsedIndex;
>    m_boostArray_t  m_array;
> };
>
>
Received on Sun Dec 12 03:14:16 2010