Re: Passing a GUI handle to a socket and Postmessage back to GUI
"Joseph M. Newcomer" <newcomer@flounder.com> wrote in message
news:qn1gr4p6at6kevefvjo92v53n759h4r0uc@4ax.com...
see below...
On Wed, 11 Mar 2009 12:13:59 -0400, "KW" <KW@there.com> wrote:
I have a MDI application with several child windows and a TCPIP server. I
want to pass the handle (of one child) to a server socket class so it can
post a message to the child window when data is received. When the data is
received by the child window, a listbox will display the data.
I have it working, but I wonder if there is a better way of passing (or
getting) the handle. I have to pass it twice to get it into the
OnReceive() Handler of CServer.cpp.
****
Define what you mean, "pass it twice to get it into the OnReceive
handler".
****
// Click a button and start the listener - couldn't get it to work in
constructor (handle not created yet?) - "SocketWindow" is UI class
void SocketWindow::OnButton1()
****
This is already indicative of some serious issues. Why are you using
those stupid
IDC_BUTTON1 names? First thing you should do after changing a control is
change its name
to something that makes sense!
****
Joseph, the "OnButton1" is a bad name. It will not be the same in the final
product.
{
sockServ = new MySocket(CWnd::m_hWnd); //Is this the best way to
get/pass the handle?
****
I have no idea, since I have no idea what your constructor is doing with
it
****
sockServ->Create(1001);
****
I hope this is not the socket number you are using. It is in the
"Well-Known Ports" range
(1..1023) and should not be used by ordinary users (even though it is
currently marked as
"unassigned", it is an exceptionally poor choice of port number. See
http://www.iana.org/assignments/port-numbers )
I will also make sure I stay clear of the port numbers through 1023. I was
not aware of this, but the 1001 was only being used for testing.
sockServ->Listen();
}
void SocketWindow::OnSockMSG(UINT wParam, long lParam)
{
CString * ptr = (CString *)wParam;
m_ListBoxAdder.AddString(*ptr);
delete(ptr);
}
****
This looks fine
****
// These 4 lines are from MySocket.h
public:
MySocket(HWND);
****
You could also use a CWnd* here
****
I am not quite sure how to use a CWnd* here.
virtual ~MySocket();
CServer client;
HWND ChildWindowHandle;
// Constructor MySocket.cpp
MySocket::MySocket(HWND MyHandle)
{
ChildWindowHandle = MyHandle;
}
// MySocket.cpp
void MySocket::OnAccept(int nErrorCode)
{
if (nErrorCode == 0)
{
this->Accept(client);
****
Why this->? It is not needed, and looks ugly
Note that this means you can only accept ONE connection, and the lifetime
of that variable
must exceed the lifetime of the connection. This is risky. Why are you
limiting yourself
in this way? It is bad design. You should create the client socket
object as needed
using new.
****
// IS THIS A PROBLEM? "client" is declared in CServer.h
client.HandleToChld = ChildWindowHandle ;
****
Is this what you mean by "passing in twice"? I don't see anything being
passed in twice
to anyone. What I see is a handle being passed to a listener class, and
then the listener
class passing it to a newly-created bound socket. This is not "twice" for
anything.
****
}
}
// CServer.cpp
void CServer::OnReceive(int nErrorCode)
{
****
And why are you not checking nErrorCode?
****
char buffer[1024] ;
memset(buffer, 0, sizeof(buffer)) ;
****
The memset is useless and should be eliminated
****
this->Receive(buffer, sizeof(buffer)) ;
****
And how do you know how many characters were received, or received
successfully? Did you
note that Receive returns a value?
int count = Receive(buffer, sizeof(buffer) - 1);
if(count <= 0)
{ /* deal with error */
...
} /* deal with error
buffer[count] = '\0';
****
CString result(buffer);
CString * ptrToData = new CString(result) ;
****
Why copy twice?
CString * ptrToData = new CString(buffer);
does the job without doing two copies!
****
Removed the first copy.
// "WM_SCKTMSG_RECV" is defined in stdAfx.h (#define WM_SCKTMSG_RECV
WM_USER + 3), executes "OnSockMSG" above
****
DO NOT EVER call a user-defined message a WM_ message. WM_ is reserved
for Microsoft.
Invent your own prefix. Many of us UWM_ or WMU_ but you could use MYSOCK_
or whatever you
want. But using WM will confuse anyone (including yourself, a year from
now) who reads
this code.
UWM_ sounds good to me and it is already changed.
It seems moderately INSANE to have put this definition stdafx.h. You
should not be adding
random definitions of your own to that file. Create another header file
for your socket
class, and IN FACT the correct place to define this value is in the header
class for your
socket class! It is NOT a global definition for every module.
The definition is now in the socket header class.
Keep you hands OFF stdafx.h except for adding #include <...> style
includes, or changing
some of the #defines to represent the correct version of the OS, or IE, or
whatever that
you are building for. DO NOT add your own definitions to it!
DO NOT use WM_USER-based values for this kind of message. WM_USER
messages can be used by
the window you are posting to for its own purposes, and using WM_USER
opens up the
possibility for total disaster, either now or in the next release of MFC.
Use WM_APP.
Better still, use Registered Window Messages
****
WM_USER will be stricken from the code. I'll use WM_APP for now. I will need
to do some research on using Registered Window Messages.
::PostMessage(HandleToChld, WM_SCKTMSG_RECV, (WPARAM)ptrToData,
(LPARAM)0);
****
If you used a CWnd* this would be
ChildWnd->PostMessage(UWM_SCKMSG_RECV, (WPARAM)ptrToData);
which reads a lot cleaner
****
I would like to use CWnd, but I am not sure how to get it.
Like this...?
CWnd * MyWindow = GetDlgItem(IDD_SOCK_WINDOW); //Within the windows'
class
//CAsyncSocket::OnReceive(nErrorCode);
}
Although this code currently works, it does not feel right (passing the
handle in the constructor??).
****
I don't see anything wrong here; I would not have used the HWND, but the
CWnd*. But there
are deeper problems in the code which I have already observed above.
****
There has to be a better way of doing this. Any suggestions?
****
As far as passing in the message sink, I don't think there's anything
wrong with what you
did, or for that matter, I don't think there is a better way. But you
need to fix the
other problems.
I am guessing that you are using CAsyncSocket. If you are using CSocket,
you need to
change to CAsyncSocket. The code looks like CAsyncSocket-style code,
which is why I'm
assuming that's what you are using.
joe
****
I am using CAsyncSocket and I will need multiple connections. Since I
removed the "this->" from "Accept(client), I should be okay, correct?
Thanks,
KW
Joseph M. Newcomer [MVP]
email: newcomer@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm