Arrays of Strings- stumbling over a very basic question

[anw]

I have a declaration like this (from a .h file):

class emlMsg
    {
  public:
    emlMsg(string From, int NumAddrs, string Addrs[], string To, string Sbj, string Msg)
        {init(From, NumAddrs, Addrs, To, Sbj, Msg);}

    void init(string From, int NumAddrs, string Addrs[], string To, string Sbj, string Msg);
    emlMsg& operator = (emlMsg& rhs){this->init(rhs.from, rhs.numAddrs, rhs.addrs,
                                                rhs.to, rhs.sbj, rhs.msg); return *this;}
    ~emlMsg();
    void dump(char* prefix="");
    string from;
    string* addrs;
    string to;
    string sbj;
    string msg;
    int numAddrs;
    };

(note the bolded declaration) that is initialized and has an operator = defined through the function init and has a destructor defined in a .cpp file thusly:

void emlMsg::init(string From, int NumAddrs, string Addrs[], string To, string Sbj, string Msg)
    {
    from= From;
    to= To;
    sbj= Sbj;
    msg= Msg;
    numAddrs= NumAddrs;
    addrs= new string[NumAddrs];
    for(int i= 0; i < NumAddrs; i++)addrs[i]= Addrs[i];
    dump("emlMsg::init: New Message");
    }

emlMsg::~emlMsg()
    {
    theLog.Write(LogDebug, "emlMsg::~emlMsg: deleting addrs\n");
    delete addrs;
    theLog.Write(LogDebug, "emlMsg::~emlMsg: done deleting addrs\n");
    }

(Note bolded assignment and delete statement)

Three questions:

1. When I delete addrs in ~emlMsg, I get an abort signal. Any idea what I could be doing wrong here?

2. When I declare the addrs like this:

string addrs[];

in the .h file, the compiler says:

error: incompatible types in assignment of 'std::string*' to 'std::string [0u]

What's wrong here?

3. A string assignment operator is overloaded, right, so string1=string2 actually copies from string2 to string1 instead of just assigning pointers?

I know I should be using vectors and will probably change over, but I'm really curious (not to mention frustrated) about this.

TIA,
anw

[AlanGRutter]

1. You need to use the array form of delete

delete [] addrs;

since you declared an array. You should also have a copy constructor.

2. There's nothing wrong with the declaration - you probably need to change your operator= - where exactly do you get the error.

3. AFAIK string1=string2 will actually make a copy of the string. But because you are dynamically allocating arrays of memory you still need your own copy constructor and operator=.

Regards
Alan

[Hobson]

1. To delete an array, you should use delete[] and not delete
2. I think that definition in this form: string addrs[] is not legal C++
3. std::string has overloaded operator=, so doing following:

std::streing s1 = "string 1", s2 = "Another string";
s1 = s2;

copies contents of s2 into s1.

And yes, you'd better use std::vector<std::string>.

Cheers

[Paul McKenzie]

2. When I declare the addrs like this:

string addrs[];

This is illegal C++. Arrays must know its dimensions at compile time.

Also, your assignment operator has a huge memory leak. Hopefully you can see what it is (hint -- what happened to the original string that was stored in the left-hand-side of the assignment of emlMsg? Whas it ever deallocated? No.)

emlMsg msg1;
//...
emlMsg msg2;
//...
// assume msg1 has some stuff in it.
msg1 = msg2; // what happened to the stuff that was originally in msg1??

I know I should be using vectors and will probably change over,

My advice -- change over to vectors now, and leave the academic exercises for later (like when you must do things like this).

Regards,

Paul McKenzie

[anw]

Urrggghhhh! delete[], of course! I knew it was something basic. So that answers question 1 & 3. As far as 2 goes, I'm pretty sure a declaration like

string addrs[];

is legal, but when I declare the string this way, then do the assignment

addrs= new string[NumAddrs];

I get a compiler error. In fact, the error

error: incompatible types in assignment of 'std::string*' to 'std::string [0u]

says the compiler even knows it's a * to [] assignment. In other words, it likes it declared as a * but not as a [], even though I'm assigning a [] to it.

[anw]

Ha! OK, the consensus is that the string addrs[] form is not legal- I bow to consensus.

As to the memory leak, you're right. I even ran it through valgrind, but here's the rub: the way it's being used, the lhs is created (without having the dynamic array assigned) then immediately set equal to another one, so it didn't show up.

[Paul McKenzie]

Ha! OK, the consensus is that the string addrs[] form is not legal- I bow to consensus.

It's not consensus, those are the rules of ANSI C++. If your compiler allows such declarations, then it is a non-standard extension that was added by your compiler.

There should be a compiler switch that would turn off these extensions, and then the compiler would have given you an error when attempting to compile the code.

Regards,

Paul McKenzie

[anw]

I thought I had done it before, but now can't find where. I use the gnu compiler, it's about as standard as you can get, I think, and I don't give it any switches to behave otherwise. I probably just had a brain fart.

I had always thought, though, that var[] and var * were equivalent. Now I know better

Thanks to all!!

anw

[TheCPUWizard]

Actually "bar[]" is closer in context to "bar * const"

[reko_t]

I thought I had done it before, but now can't find where.

Maybe you had done something like this before:

string addrs[] = { "localhost", "example.com", "etc" };

"type name[]" declaration is fine when you initialize the array in the same statement.

I had always thought, though, that var[] and var * were equivalent. Now I know better

Not quite, although depending on context they can be used interchangeably. For example as a function parameter it doesn't make a difference, except when multidimensional arrays are concerned.