Memory Leaks - Can you help me find them in ths snippet

Discussion in 'C++' started by nmehring@gmail.com, Jan 28, 2008.

  1. Guest

    I am writing some c++ code that interacts with a native C library and
    have to do some dynamic memory allocation to support it. I am getting
    memory leaks and I think this piece of code is the culprit. Can
    anyone tell me what I might be doing wrong?

    char **columns;
    columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));

    if (NULL == columns) return FALSE; //memory allocation failed

    //allocate space for the column names in the char**
    int column_index;
    for (column_index = 0; column_index < lColumnCount; column_index++)
    {
    // Allocate for the maximum owner.table.column notation
    columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);

    if (NULL == columns[column_index])
    {
    delete[] columns; //memory allocation failed
    return FALSE;
    }
    }

    //put the data in the char**
    for (column_index = 0; column_index < lColumnCount; column_index++)
    {
    strcpy (columns[column_index], CStringColumnName[column_index]);
    }

    //do some stuff with columns
    ........

    delete[] columns;





    Nicole
    , Jan 28, 2008
    #1
    1. Advertising

  2. wrote:
    > I am writing some c++ code that interacts with a native C library and
    > have to do some dynamic memory allocation to support it. I am getting
    > memory leaks and I think this piece of code is the culprit. Can
    > anyone tell me what I might be doing wrong?
    >
    > char **columns;
    > columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));
    > [..]
    >
    > delete[] columns;


    Don't EVER mix 'malloc/free' and 'new/delete' (or 'new[]/delete[]').
    Those come in pairs, use them in pairs.

    V
    --
    Please remove capital 'A's when replying by e-mail
    I do not respond to top-posted replies, please don't ask
    Victor Bazarov, Jan 28, 2008
    #2
    1. Advertising

  3. * :
    > I am writing some c++ code that interacts with a native C library and
    > have to do some dynamic memory allocation to support it. I am getting
    > memory leaks and I think this piece of code is the culprit. Can
    > anyone tell me what I might be doing wrong?
    >
    > char **columns;
    > columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));
    >
    > if (NULL == columns) return FALSE; //memory allocation failed
    >
    > //allocate space for the column names in the char**
    > int column_index;
    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > // Allocate for the maximum owner.table.column notation
    > columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);
    >
    > if (NULL == columns[column_index])
    > {
    > delete[] columns; //memory allocation failed
    > return FALSE;
    > }
    > }
    >
    > //put the data in the char**
    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > strcpy (columns[column_index], CStringColumnName[column_index]);
    > }
    >
    > //do some stuff with columns
    > .......
    >
    > delete[] columns;


    Using malloc for allocation and delete[] for deallocation is Undefined
    Behavior.

    Code below would be better expressed using ScopeGuard.

    But in order not to bring in that library (disclaimer: code not touched
    by compiler's hands):

    class Columns
    {
    private:
    std::vector<char*> myColumns;

    void dealloc()
    {
    for( size_t i = 0; i < myColumns.size(); ++i )
    {
    delete[] myColumns;
    }
    }

    public:
    Columns(): myColumns( ::lColumnCount )
    {
    try
    {
    for( int i = 0; i < lColumnCount; ++i )
    {
    myColumns = new char[SE_QUALIFIED_COLUMN_LEN];
    strcpy( myColumns, ::CStringColumnsName );
    }
    }
    catch( std::bad_alloc const& )
    {
    dealloc();
    throw;
    }
    }

    ~Columns() { dealloc(); }

    char** pFirst() { return &myColumns[0]; }
    };


    bool foo()
    {
    try
    {

    Columns columns;
    someApiFunc( columns.pFirst() );
    return true;
    }
    catch( ... )
    {
    return false;
    }
    }

    foo() could be simpler if it signalled failure via exception rather than
    via a bool result.


    Cheers & hth.,

    - Alf



    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Jan 28, 2008
    #3
  4. Guest

    Thanks, I was wondering if that was a problem. I went ahead and made
    the change, but still have memory leaks. Anyone have any ideas for
    me? I know it hits the "free(columns)" at the end of the code
    snippet.

    char **columns;
    columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));

    if (NULL == columns) return FALSE; //memory allocation failed

    //allocate space for the column names in the char**
    int column_index;
    for (column_index = 0; column_index < lColumnCount; column_index++)
    {
    // Allocate for the maximum owner.table.column notation
    columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);

    if (NULL == columns[column_index])
    {
    free(columns); //memory allocation failed
    return FALSE;
    }
    }


    //put the data in the char**
    for (column_index = 0; column_index < lColumnCount; column_index++)
    {
    strcpy (columns[column_index], CStringColumnName[column_index]);
    }


    //do some stuff with columns
    ........

    free(columns);
    , Jan 28, 2008
    #4
  5. Pete Becker Guest

    On 2008-01-28 12:35:34 -0500, "" <> said:

    > // Allocate for the maximum owner.table.column notation
    > columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);


    The memory allocated here never gets freed.

    --
    Pete
    Roundhouse Consulting, Ltd. (www.versatilecoding.com) Author of "The
    Standard C++ Library Extensions: a Tutorial and Reference
    (www.petebecker.com/tr1book)
    Pete Becker, Jan 28, 2008
    #5
  6. * :
    > Thanks, I was wondering if that was a problem. I went ahead and made
    > the change, but still have memory leaks. Anyone have any ideas for
    > me?


    Perhaps your news-server hasn't propagated all replies in the thread.

    I'm sure you'll find them in Google Groups, though.


    Cheers & hth.,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Jan 28, 2008
    #6
  7. Jim Langston Guest

    wrote:
    > Thanks, I was wondering if that was a problem. I went ahead and made
    > the change, but still have memory leaks. Anyone have any ideas for
    > me? I know it hits the "free(columns)" at the end of the code
    > snippet.


    count the malloc/news you make and the free/deletes.

    > char **columns;
    > columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));


    Here's 1 malloc

    > if (NULL == columns) return FALSE; //memory allocation failed
    >
    > //allocate space for the column names in the char**
    > int column_index;
    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > // Allocate for the maximum owner.table.column notation
    > columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);


    Here's another malloc that's going to happen lColumnCount times.

    > if (NULL == columns[column_index])
    > {
    > free(columns); //memory allocation failed


    This one is only on case of error, lets ignore this for now

    > return FALSE;
    > }
    > }
    >
    >
    > //put the data in the char**
    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > strcpy (columns[column_index], CStringColumnName[column_index]);
    > }
    >
    >
    > //do some stuff with columns
    > .......
    >
    > free(columns);


    And here is one free. What about the lColumnCount frees you need to do?
    You need a free/delete for every malloc/new you do. Before you do this
    free(columns) you need to:

    for (column_index = 0; column_index < lColumnCount; column_index++)
    {
    free( columns[column_index] );
    }

    --
    Jim Langston
    Jim Langston, Jan 28, 2008
    #7
  8. Guest

    Thanks everyone, that solved the problem. Here is the corrected code:

    char **columns;
    columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));

    if (NULL == columns) return FALSE; //memory allocation failed

    //allocate space for the column names in the char**
    int column_index;
    for (column_index = 0; column_index < lColumnCount; column_index++)
    {
    // Allocate for the maximum owner.table.column notation
    columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);

    if (NULL == columns[column_index])
    {
    free(columns); //memory allocation failed
    return FALSE;
    }
    }

    //put the data in the char**
    for (column_index = 0; column_index < lColumnCount; column_index++)
    {
    strcpy (columns[column_index], CStringColumnName[column_index]);
    }


    //do some stuff with columns
    ........

    for( int i = 0; i < lColumnCount; ++i )
    {
    free(columns);
    }
    free(columns);
    , Jan 28, 2008
    #8
  9. * :
    > Thanks everyone, that solved the problem. Here is the corrected code:
    >
    > char **columns;
    > columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));
    >
    > if (NULL == columns) return FALSE; //memory allocation failed
    >
    > //allocate space for the column names in the char**
    > int column_index;
    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > // Allocate for the maximum owner.table.column notation
    > columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);
    >
    > if (NULL == columns[column_index])
    > {
    > free(columns); //memory allocation failed
    > return FALSE;


    In this case you have a memory leak.


    > }
    > }
    >
    > //put the data in the char**
    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > strcpy (columns[column_index], CStringColumnName[column_index]);
    > }
    >
    >
    > //do some stuff with columns
    > .......
    >
    > for( int i = 0; i < lColumnCount; ++i )
    > {
    > free(columns);
    > }
    > free(columns);



    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Jan 28, 2008
    #9
  10. Default User Guest

    wrote:

    > Thanks everyone, that solved the problem. Here is the corrected code:


    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > // Allocate for the maximum owner.table.column notation
    > columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);
    >
    > if (NULL == columns[column_index])
    > {
    > free(columns); //memory allocation failed
    > return FALSE;
    > }
    > }


    What happens if the allocation failure occurs when i != 0?




    Brian
    Default User, Jan 28, 2008
    #10
  11. Daniel T. Guest

    "" <> wrote:

    > I am writing some c++ code that interacts with a native C library and
    > have to do some dynamic memory allocation to support it. I am getting
    > memory leaks and I think this piece of code is the culprit. Can
    > anyone tell me what I might be doing wrong?


    Yes. Remove the mallocs and deletes. Use std::vectors instead.

    vector< vector< char > > columns( lColumnCount,
    SE_QUALIFIED_COLUMN_LEN );


    //put the data in the char**
    for ( int column_index = 0; column_index < lColumnCount;
    column_index++)
    {
    strcpy (&columns[column_index].front(),
    CStringColumnName[column_index]);
    }
    //do some stuff with columns
    //.......

    Or maybe better:

    vector< string > columns( lColumnCount );


    //put the data in the char**
    for ( int column_index = 0; column_index < lColumnCount;
    column_index++)
    {
    columns[column_index] = CStringColumnName[column_index];
    }

    //do some stuff with columns
    //.......

    Or maybe better still:

    vector< string > columns;

    copy( CStringColumnName, CStringColumnName + lColumnCount,
    back_inserter( columns ) );

    //do some stuff with columns
    //.......
    Daniel T., Jan 29, 2008
    #11
  12. Daniel T. Guest

    "" <> wrote:

    > Thanks everyone, that solved the problem. Here is the corrected code:


    The code is better, but still not correct. I say again, save yourself
    the headache and use vectors and/or strings. Both vectors and strings
    can be used with C libraries and you don't have to worry about memory
    leaks when you use them.

    vector< string > columns;
    copy( CStringColumnName, CStringColumnName + lColumnCount,
    back_inserter( columns ) );

    //do some stuff with columns
    //.......

    > char **columns;
    > columns = (CHAR **)malloc (lColumnCount * sizeof (CHAR *));
    >
    > if (NULL == columns) return FALSE; //memory allocation failed
    >
    > //allocate space for the column names in the char**
    > int column_index;
    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > // Allocate for the maximum owner.table.column notation
    > columns[column_index] = (char *)malloc(SE_QUALIFIED_COLUMN_LEN);
    >
    > if (NULL == columns[column_index])
    > {
    > free(columns); //memory allocation failed
    > return FALSE;
    > }
    > }
    >
    > //put the data in the char**
    > for (column_index = 0; column_index < lColumnCount; column_index++)
    > {
    > strcpy (columns[column_index], CStringColumnName[column_index]);
    > }
    >
    >
    > //do some stuff with columns
    > .......
    >
    > for( int i = 0; i < lColumnCount; ++i )
    > {
    > free(columns);
    > }
    > free(columns);
    Daniel T., Jan 29, 2008
    #12
  13. Guest

    Thank you, I will correct that additional leak if i>0

    The reason I am using these datatypes is because I am utilizing this
    3rd party library method:

    extern LONG SDEAPI SE_stream_update_row (SE_STREAM stream,
    const ACHAR *table,
    LONG *sde_row_id,
    SHORT num_columns,
    const ACHAR **columns);

    So I figured I had to use the char data type.

    Nicole
    , Jan 29, 2008
    #13
  14. * :
    > Thank you, I will correct that additional leak if i>0
    >
    > The reason I am using these datatypes is because I am utilizing this
    > 3rd party library method:
    >
    > extern LONG SDEAPI SE_stream_update_row (SE_STREAM stream,
    > const ACHAR *table,
    > LONG *sde_row_id,
    > SHORT num_columns,
    > const ACHAR **columns);
    >
    > So I figured I had to use the char data type.


    You will have to supply an array of char pointers.

    A std::vector<std::string>, as suggested in the article you're replying,
    is not that.

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Jan 29, 2008
    #14
  15. Daniel T. Guest

    "" <> wrote:

    > Thank you, I will correct that additional leak if i>0
    >
    > The reason I am using these datatypes is because I am utilizing this
    > 3rd party library method:
    >
    > extern LONG SDEAPI SE_stream_update_row (SE_STREAM stream,
    > const ACHAR *table,
    > LONG *sde_row_id,
    > SHORT num_columns,
    > const ACHAR **columns);
    >
    > So I figured I had to use the char data type.


    Sorry, I thought you were using each column individually. i.e., I
    thought the API was asking for char* and was being called multiple
    times, not char** and called once.

    However, I still recommend you use vectors rather than allocating the
    memory yourself and hoping for the best.

    Something like this would do nicely:

    vector< vector< char > >
    block( lColumnCount, SE_QUALIFIED_COLUMN_LEN );

    for ( int i = 0; i < block.size(); ++i )
    {
    strcpy( &block.front(), CStringColumnName );
    }

    vector< char* > columns( block.size() );
    for ( int i = 0; i != block.size(); ++i )
    {
    columns = &block.front();
    }
    SE_stream_update_row( /* other params */, &columns[0] );
    Daniel T., Jan 29, 2008
    #15
  16. * Daniel T.:
    > "" <> wrote:
    >
    >> Thank you, I will correct that additional leak if i>0
    >>
    >> The reason I am using these datatypes is because I am utilizing this
    >> 3rd party library method:
    >>
    >> extern LONG SDEAPI SE_stream_update_row (SE_STREAM stream,
    >> const ACHAR *table,
    >> LONG *sde_row_id,
    >> SHORT num_columns,
    >> const ACHAR **columns);
    >>
    >> So I figured I had to use the char data type.

    >
    > Sorry, I thought you were using each column individually. i.e., I
    > thought the API was asking for char* and was being called multiple
    > times, not char** and called once.
    >
    > However, I still recommend you use vectors rather than allocating the
    > memory yourself and hoping for the best.
    >
    > Something like this would do nicely:
    >
    > vector< vector< char > >
    > block( lColumnCount, SE_QUALIFIED_COLUMN_LEN );
    >
    > for ( int i = 0; i < block.size(); ++i )
    > {
    > strcpy( &block.front(), CStringColumnName );
    > }
    >
    > vector< char* > columns( block.size() );
    > for ( int i = 0; i != block.size(); ++i )
    > {
    > columns = &block.front();
    > }
    > SE_stream_update_row( /* other params */, &columns[0] );


    This is a better idea than the code I posted: it uses a little more
    memory but is safer (wrt. maintainance) and shorter and just more clear.

    However, the 'block' constructor arguments need to be fixed, and the
    whole thing needs to be put in a try-catch in order to conform to the
    OP's boolean return.

    I'd just default-initialize the inner vectors, and .resize() them in the
    string copy loop.


    Cheers,

    - Alf


    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Jan 29, 2008
    #16
  17. Daniel T. Guest

    In article <>,
    "Alf P. Steinbach" <> wrote:

    > * Daniel T.:
    > > "" <> wrote:
    > >
    > >> Thank you, I will correct that additional leak if i>0
    > >>
    > >> The reason I am using these datatypes is because I am utilizing this
    > >> 3rd party library method:
    > >>
    > >> extern LONG SDEAPI SE_stream_update_row (SE_STREAM stream,
    > >> const ACHAR *table,
    > >> LONG *sde_row_id,
    > >> SHORT num_columns,
    > >> const ACHAR **columns);
    > >>
    > >> So I figured I had to use the char data type.

    > >
    > > Sorry, I thought you were using each column individually. i.e., I
    > > thought the API was asking for char* and was being called multiple
    > > times, not char** and called once.
    > >
    > > However, I still recommend you use vectors rather than allocating the
    > > memory yourself and hoping for the best.
    > >
    > > Something like this would do nicely:
    > >
    > > vector< vector< char > >
    > > block( lColumnCount, SE_QUALIFIED_COLUMN_LEN );
    > >
    > > for ( int i = 0; i < block.size(); ++i )
    > > {
    > > strcpy( &block.front(), CStringColumnName );
    > > }
    > >
    > > vector< char* > columns( block.size() );
    > > for ( int i = 0; i != block.size(); ++i )
    > > {
    > > columns = &block.front();
    > > }
    > > SE_stream_update_row( /* other params */, &columns[0] );

    >
    > This is a better idea than the code I posted: it uses a little more
    > memory but is safer (wrt. maintainance) and shorter and just more clear.
    >
    > However, the 'block' constructor arguments need to be fixed, and the
    > whole thing needs to be put in a try-catch in order to conform to the
    > OP's boolean return.


    Accepted about the try catch block, but what is wrong with the
    constructor arguments, did I reverse them or something?
    >
    > I'd just default-initialize the inner vectors, and .resize() them in the
    > string copy loop.


    Another idea would be to allocate the block as a single vector<char>
    rather than a vector of vectors, then partition the block into the
    columns vector, like so:

    vector< char > block( lColumnCount * SE_QUALIFIED_COLUMN_LEN );
    vector< char* > columns( lColumnCount );
    for ( int i = 0, j = 0; i < lColumnCount;
    ++i, j += SE_QUALIFIED_COLUMN_LEN ) {
    columns = &block[j];
    }
    // and so on.
    Daniel T., Jan 29, 2008
    #17
  18. * Daniel T.:
    > In article <>,
    > "Alf P. Steinbach" <> wrote:
    >
    >> * Daniel T.:
    >>> "" <> wrote:
    >>>
    >>>> Thank you, I will correct that additional leak if i>0
    >>>>
    >>>> The reason I am using these datatypes is because I am utilizing this
    >>>> 3rd party library method:
    >>>>
    >>>> extern LONG SDEAPI SE_stream_update_row (SE_STREAM stream,
    >>>> const ACHAR *table,
    >>>> LONG *sde_row_id,
    >>>> SHORT num_columns,
    >>>> const ACHAR **columns);
    >>>>
    >>>> So I figured I had to use the char data type.
    >>> Sorry, I thought you were using each column individually. i.e., I
    >>> thought the API was asking for char* and was being called multiple
    >>> times, not char** and called once.
    >>>
    >>> However, I still recommend you use vectors rather than allocating the
    >>> memory yourself and hoping for the best.
    >>>
    >>> Something like this would do nicely:
    >>>
    >>> vector< vector< char > >
    >>> block( lColumnCount, SE_QUALIFIED_COLUMN_LEN );
    >>>
    >>> for ( int i = 0; i < block.size(); ++i )
    >>> {
    >>> strcpy( &block.front(), CStringColumnName );
    >>> }
    >>>
    >>> vector< char* > columns( block.size() );
    >>> for ( int i = 0; i != block.size(); ++i )
    >>> {
    >>> columns = &block.front();
    >>> }
    >>> SE_stream_update_row( /* other params */, &columns[0] );

    >> This is a better idea than the code I posted: it uses a little more
    >> memory but is safer (wrt. maintainance) and shorter and just more clear.
    >>
    >> However, the 'block' constructor arguments need to be fixed, and the
    >> whole thing needs to be put in a try-catch in order to conform to the
    >> OP's boolean return.

    >
    > Accepted about the try catch block, but what is wrong with the
    > constructor arguments, did I reverse them or something?


    Well, I was wrong, but on a higher level (see below) I was right.

    What I remembered was my practical experience with one particular
    compiler, which would not accept the above, while the standard does
    require that it is accepted, via a very very subtle special case rule.

    Consider

    #include <vector>
    #include <iostream>

    int main()
    {
    using namespace std;
    vector< vector<int> > v( 3, 7 ); // The item discussed here.
    cout << v.size() << ", " << v[0].size() << endl;
    }

    Trying to compile, various compilers:


    <g++>
    V:\> gnuc vc_project.cpp

    V:\> a
    3, 7
    </g++>


    <msvc 7.1>
    V:\> msvc vc_project.cpp -o b.exe
    vc_project.cpp
    C:\Program Files\Microsoft Visual Studio .NET
    2003\Vc7\include\vector(357) : error C2664: 'std::vector<_Ty>::_Construct_
    n' : cannot convert parameter 2 from 'int' to 'const std::vector<_Ty> &'
    with
    [
    _Ty=std::vector<int>
    ]
    and
    [
    _Ty=int
    ]
    Reason: cannot convert from 'int' to 'const std::vector<_Ty>'
    with
    [
    _Ty=int
    ]
    Constructor for class 'std::vector<_Ty>' is declared 'explicit'
    with
    [
    _Ty=int
    ]
    C:\Program Files\Microsoft Visual Studio .NET
    2003\Vc7\include\vector(343) : see reference to function template
    instantiation 'void
    std::vector<_Ty>::_Construct<_Iter>(_Iter,_Iter,std::_Int_iterator_tag)'
    being compiled
    with
    [
    _Ty=std::vector<int>,
    _Iter=int
    ]
    vc_project.cpp(7) : see reference to function template
    instantiation 'std::vector<_Ty>::vector<int>(_Iter,_Iter)
    ' being compiled
    with
    [
    _Ty=std::vector<int>,
    _Iter=int
    ]

    V:\>_
    <msvc 7.1>


    <comeau online>
    Your Comeau C/C++ test results are as follows:

    Comeau C/C++ 4.3.9 (Mar 27 2007 17:24:47) for ONLINE_EVALUATION_BETA1
    Copyright 1988-2007 Comeau Computing. All rights reserved.
    MODE:strict errors C++ C++0x_extensions


    In strict mode, with -tused, Compile succeeded (but remember, the Comeau
    online compiler does not link).
    Compiled with C++0x extensions enabled.
    </comeau online>


    So which compiler is right? Comeau usually is, and is also this time.

    What happens is that instead of the explicit constructor taking size
    argument, the templated constructor taking iterators is invoked for the
    outer vector, and §23.1.1/9, general requirements for containers, says
    that when invoked with integral argument type instead of the expected
    some iterator type, it shall behave the same as (in this case) vector<
    vector<int> >( static_cast<size_type>(3), static_cast< vector<int> >( 7
    ) ), or in other words effectively the same as

    vector< vector<int> > v( 3, vector<int>( 7 ) );

    But I wouldn't rely on the compiler / library implementation being smart
    enough to figure that out.

    I'd write it as what the standard requires it ends up as, namely this
    latter declaration, because then everybody understand what it means,
    including the author, the MSVC 7.1 compiler, and maintenance... :)

    Cheers,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Jan 29, 2008
    #18
  19. James Kanze Guest

    On Jan 29, 8:35 pm, "Alf P. Steinbach" <> wrote:
    > * Daniel T.:


    [...]
    > >>> vector< vector< char > >
    > >>> block( lColumnCount, SE_QUALIFIED_COLUMN_LEN );


    [...]
    > > Accepted about the try catch block, but what is wrong with the
    > > constructor arguments, did I reverse them or something?


    > Well, I was wrong, but on a higher level (see below) I was right.


    > What I remembered was my practical experience with one
    > particular compiler, which would not accept the above, while
    > the standard does require that it is accepted, via a very very
    > subtle special case rule.


    Note that there has been some discussion as to whether the fact
    that the above is legal is due to an error in the wording of the
    special case. The special case was designed to handle things
    like:

    std::vector< int > v( 10, 43 ) ;

    Note that the best match for this constructor is the template
    constructor taking (nominally) two iterators, which is probably
    not what the user expected. (There is also a constructor taking
    a size_t and a T---in this case an int. But of course, that
    involves a conversion of int to size_t, where as the template
    constructor taking two "iterators" is an exact match---the
    template mechanism can't tell that int's can't be iterators.)

    The result is that a special case was added to make this
    constructor act as expected. I don't think that the original
    intent of the wording was to also make Daniel's constructor well
    formed (and in fact, as you point out, some compilers don't
    accept it).

    I believe that there was even a DR, although I'm not sure. At
    any rate, I just checked the current draft: the wording of the
    special case has been changed to make Daniel's version illegal
    (so you're not really wrong, just living in the future:).

    > Consider


    > #include <vector>
    > #include <iostream>


    > int main()
    > {
    > using namespace std;
    > vector< vector<int> > v( 3, 7 ); // The item discussed here.
    > cout << v.size() << ", " << v[0].size() << endl;
    > }


    > Trying to compile, various compilers:


    It's a library issue, more than a compiler issue. If the two
    arguments have the same type, and the first is not size_t, then
    the template function taking two iterators is a better match.
    Beyond that, it's up to the library to make it work "as if" the
    (size_t, T) constructor had been called, with the arguments
    "static_cast" to the correct type. (The latest draft changes
    this, and says that only the first argument should be
    static_cast to a size_type.) The 1998 version of the standard
    also contains a note (non-normative) suggesting a simple
    technique to implement this---a library which uses that
    technique, however, will fail to compile Daniel's code.

    My guess is that for the compilers where this code failed to
    compile, the library implementation is based on that note.

    > So which compiler is right? Comeau usually is, and is also
    > this time.


    For the moment:). (Also, just curious, but does anyone know
    what implementation of the library the Comeau on line uses?)

    > What happens is that instead of the explicit constructor taking size
    > argument, the templated constructor taking iterators is invoked for the
    > outer vector, and §23.1.1/9, general requirements for containers, says
    > that when invoked with integral argument type instead of the expected
    > some iterator type, it shall behave the same as (in this case) vector<
    > vector<int> >( static_cast<size_type>(3), static_cast< vector<int> >( 7
    > ) ), or in other words effectively the same as


    > vector< vector<int> > v( 3, vector<int>( 7 ) );


    > But I wouldn't rely on the compiler / library implementation
    > being smart enough to figure that out.


    > I'd write it as what the standard requires it ends up as,
    > namely this latter declaration, because then everybody
    > understand what it means, including the author, the MSVC 7.1
    > compiler, and maintenance... :)


    And libraries which will implement C++0x:).

    Totally agreed (and I would be even if the standards committee
    had decided to live with the original text, on the grounds that
    even if it wasn't what was meant, changing it now would break
    too much codde).

    --
    James Kanze (GABI Software) email:
    Conseils en informatique orientée objet/
    Beratung in objektorientierter Datenverarbeitung
    9 place Sémard, 78210 St.-Cyr-l'École, France, +33 (0)1 30 23 00 34
    James Kanze, Jan 30, 2008
    #19
  20. * James Kanze -> Alf P. Steinbach:
    >
    > I believe that there was even a DR, although I'm not sure. At
    > any rate, I just checked the current draft: the wording of the
    > special case has been changed to make Daniel's version illegal
    > (so you're not really wrong, just living in the future:).


    Heh. :)


    Cheers,

    - Alf

    --
    A: Because it messes up the order in which people normally read text.
    Q: Why is it such a bad thing?
    A: Top-posting.
    Q: What is the most annoying thing on usenet and in e-mail?
    Alf P. Steinbach, Jan 30, 2008
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Phil Powell
    Replies:
    12
    Views:
    737
    Phil Powell
    Feb 1, 2004
  2. Durgesh Sharma
    Replies:
    5
    Views:
    893
    E. Robert Tisdale
    Dec 21, 2004
  3. Divick

    How to find memory leaks?

    Divick, Sep 19, 2005, in forum: C++
    Replies:
    3
    Views:
    361
    Divick
    Sep 21, 2005
  4. Replies:
    0
    Views:
    360
  5. Marco Hornung

    find memory leaks in running program

    Marco Hornung, Dec 7, 2010, in forum: Python
    Replies:
    1
    Views:
    313
    shearichard
    Dec 7, 2010
Loading...

Share This Page