Joseph M. Newcomer schrieb:
Spotted another serious problem, I think. See below...
On Mon, 07 Apr 2008 22:31:47 -0400, Joseph M. Newcomer <newcomer@flounder.com> wrote:
See below...
On Mon, 07 Apr 2008 20:10:16 +0200, Sven Eichenm?ller <sven@eichenmueller.de> wrote:
David Wilkinson schrieb:
? wrote:
But these functions (bootServer & initServer) are declared in a header
file which I included.
Declared, yes. Declarations have to do with compiling. Your code is
compiling correctly.
You are seeing linker errors, which means that the missing functions are
not defined in any of the compiled translation units. You are missing an
implementation file (.cpp) or library (.lib) file in your project.
Hey thank you,
I think that was the problem.
But now I've got a new problem. This is the output.
--------------------------
..\serverwin32.cpp(41) : error C2664: 'CreateThread' : cannot convert
parameter 3 from 'DWORD (__stdcall *)(dpws *)' to 'LPTHREAD_START_ROUTINE'
None of the functions with this name in scope match the target type
..\serverwin32.cpp(65) : error C2440: '=' : cannot convert from 'LPVOID'
to 'dpws *'
Conversion from 'void*' to pointer to non-'void' requires an
explicit cast
..\serverwin32.cpp(75) : error C2664: 'CreateThread' : cannot convert
parameter 3 from 'DWORD (__stdcall *)(dpws *)' to 'LPTHREAD_START_ROUTINE'
None of the functions with this name in scope match the target type
--------------------------
****
If you are using MFC, you must NOT use CreateThread, or _beginthread/_beginthreadex; you
MUST, ABSOLUTELY POSITIVELY use AfxBeginThread.
Note that at least part of the reason that these compiled in a console app was that you
probably had them in a .c file, and the C language is very slovenly about type matching.
In .cpp files, you are using the C++ language, which does not forgive your gratuitous
violation of the type system. Since you failed to follow the documentation, you get an
error message, because you weren't allowed to do that, but the C language lets you get
away with it. C++, correctly, will not forgive such errors.
****
The code in which these functions appear is:
--------------------------
HANDLE bootThread = NULL;
static DWORD WINAPI runServer(struct dpws *);
static DWORD WINAPI serve(struct dpws *);
****
Did you read the documentation? What is the type of the argument shown by the
specification? Is it struct dpws*? Or is it void* (LPVOID)? If it is specified as
void*/LPVOID, why, exactly, did you think it made sense to change it to something else?
The documentation is there for a reason, and you are expected to read it and follow it.
****
int initServer(struct dpws *dpws)
{
int status = dpws_server_init(dpws, NULL);
if (status)
fprintf(stderr, dpws_get_error_msg(dpws));
****
Note that ALL instances of fprintf must be deleted from Windows apps; they are completely
meaningless. You must also not pop up MessageBoxes from secondary threads. You probably
have to rethink what you are doing to fit into a window-app context
****
return status;
}
int bootServer(struct dpws *dpws)
{
bootThread = CreateThread(NULL, 0, runServer, dpws, 0, NULL);
****
This code is not correct. You must NOT call CreateThread directly if you are using the C
runtimes, which you clearly are. You MUST use _beginthread or _beginthreadex to start
threads.
****
And in an MFC app, you MUST use AfxBeginThread
****
****
if (bootThread == NULL) {
fprintf (stderr, "Thread Creation ERROR (%d) - bootServer()\n",
GetLastError());
return -1;
****
I'm curious why you are using odd values like -1 and 0 and an int return type, when you
should be using a BOOL returning TRUE for success and FALSE for error
****
}
return 0;
}
void stopServer()
{
dpws_stop_server(1000);
// wait for the master thread to exit
WaitForSingleObject(bootThread, 2000);
CloseHandle(bootThread);
}
static DWORD WINAPI runServer(struct dpws * m_dpws)
****
The parameter is CLEARLY stated in the documentation as being an LPVOID; why would you
change it to something that violates the specification? And WHY are you using the "m_"
notation for a PARAMETER????? This is reserved for class members of a C++ class and would
not be used for any other purpose!!! If you don't know how to use these notations, DO
NOT USE THEM!!!! Using them incorrectly is far worse than not using them at all.
****
{
HANDLE hThread;
struct dpws * s_dpws;
int status = DPWS_OK;
while (TRUE) {
/* Allocate a DPWS runtime structure for the next serve task */
s_dpws = HeapAlloc(GetProcessHeap(), 0, sizeof(struct dpws));
****
Why call HeapAlloc when malloc or new would do the job just fine? Actually, I'm surprised
this compiles at all, because HeapAlloc returns an LPVOID but the assignment requires a
downcast to match the type.
****
status = dpws_accept_thr(m_dpws, s_dpws);
if (status) {
if (status != DPWS_ERR_SERVER_STOPPED)
fprintf(stderr, dpws_get_error_msg(m_dpws));
****
Note that you are using fprintf, a member of the C library suite, but you are using
CreateThread, so certain C runtime initializations will not be performed. Maybe this will
work, and maybe it won't, but it is not guaranteed
****
goto exit;
****
goto? Is this an exercise in retro programming style?
****
}
/* spawn a new thread */ // NOTE: implement a pool instead
hThread = CreateThread(NULL, 0, serve, s_dpws, 0, NULL);
***
_beginthreadex
****
if (hThread == NULL) {
fprintf (stderr, "Thread Creation ERROR (%d) - runServer()\n",
status);
****
And where is status set to a meaningful value? Thie correct code would be
if(hThread == NULL)
{
status = GetLastError();
_tfprintf(stderr, _T("Thread creation ERROR (%d) - runServer()\n"),
status);
}
You should be programming Unicode-aware, and ignoring the existence of 8-bit character
literals and the char datatype
****
goto exit;
****
Why is this not simply
return 0;
instead of using the rather quaint 'goto'?
****
}
else
CloseHandle(hThread);
}
exit:
// dpws_shutdown() is performed by the DLL
****
I wonder if you have that weird belief that you should have only one "return" in a
function. Note that if you subscribe to this, you must also eliminate the 'goto' because
it is a far WORSE violation of good programming style. Essentially, if you use a 'goto',
you need to rethink how you are writing your code. This construct should be considered as
dead.
****
return 0;
}
static DWORD WINAPI serve(struct dpws * dpws)
{
dpws_serve(dpws);
dpws_end(dpws);
HeapFree(GetProcessHeap(), 0, dpws);
****
Do the two functions represent thread starts? Where is serve() called and what is its
relationship to the thread functions shown above? How does it know that it can safely
delete the object?
****
return 0;
}
--------------------------
How can I fix it?
Thanks. Sven
Joseph M. Newcomer [MVP]
email: newcomer@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
Joseph M. Newcomer [MVP]
email: newcomer@flounder.com
Web: http://www.flounder.com
MVP Tips: http://www.flounder.com/mvp_tips.htm
I really thank you for that detailed explanation. But what I only did is
and copy it into the MFC source code. My abilities are not good enough