Re: [GnomeMeeting-devel-list] [PATCH] private (and object-like) text chat

[PUYDT Julien <julien puydt laposte net>]

On mar, 2004-04-06 at 14:56, Damien Sandras wrote:
> 1) The chat window is a GtkWidget which is part of the GmWindow. It
> means that you should have a pointer to that GtkWidget inside GmWindow
> and that you should not have functions like GnomeMeeting::Process
> ()->GetTextChatWindow (). Such functions are only for top-level windows,
> not for internal widgets inside those windows.

Well, there was a GetTextChat function, you know... I just renamed it to
describe what it does.

> 2) +  /* delete (chat_window); unneeded: embedded in the main window! */
> If it is unneeded, why don't you remove the line instead of committing
> it to CVS? ;)

Two reasons:
1) that's documentation;
2) that will avoid some fool to try to free it. ;-)

> 3) +  /* the rest of the structure is automatically freed:
> +   * the main window has the chat_window, and the chat_window has them
> +   */
> +}
> 
> That's not right, how could it be automatically freed? There is a leak
> here.

You're partly right: I forgot to free the GmTextChat!

But for the rest: when the main window is freed, it frees its children ;
the chat window is one of those. The text view is a child of the chat
window, and the text buffer a child of the text view... 

> 4) Please don't forget to add forward declarations in C code at the top
> of the file.

Hmmmm... ok, I'll add them also for the other functions that lack in in
src/chat_window.cpp.

Snark