求帮优化代码....给出建议的给分~~

UINT ScanProc(LPVOID lpParam)

{

	CString IP=((IPData*)lpParam)->IP;

	SOCKET sd;

	if((sd=socket(AF_INET,SOCK_STREAM,IPPROTO_IP))==INVALID_SOCKET)

	{

		++::g_Complate;

		--::g_ActiveThread;

		return -1;

	}


	unsigned long flag = 1; 	

	if ((ioctlsocket(sd, FIONBIO, &flag) != 0))

	{

		++::g_Complate;

		--::g_ActiveThread;

		return -2;

	}

	SOCKADDR_IN sin;

	sin.sin_addr.S_un.S_addr=inet_addr(IP);

	sin.sin_family=AF_INET;

	sin.sin_port=htons(::g_Port);


	connect(sd,(SOCKADDR*)&sin,sizeof(sin));



	struct fd_set mask;			

	FD_ZERO(&mask);

	FD_SET(sd, &mask);


	struct timeval timeout;

	timeout.tv_sec = ::g_TimeOut/1000;

	timeout.tv_usec =::g_TimeOut%1000;


	if(select(0, NULL, &mask, NULL, &timeout)<=0)

	{

		++::g_Complate;

		--::g_ActiveThread;

		closesocket(sd);

		return -3;

	}

	else

	{

		char cmd[513],cmd1[513];

		memset(cmd,0,10);

		memset(cmd1,0,10);

		cmd[0]=5;

		cmd[1]=2;

		cmd[2]=0;

		cmd[3]=2;


		if(send(sd,cmd,4,0)!=4)

		{

			++::g_Complate;

			--::g_ActiveThread;

			closesocket(sd);

			return -4;

		}

		Sleep(200);


		if(recv(sd,cmd1,2,0)!=2)

		{

			++::g_Complate;

			--::g_ActiveThread;

			closesocket(sd);

			return -5;

		}

		int index;

		WaitForSingleObject(g_hEvent,INFINITE);

		index=g_pDlg->m_result.GetItemCount();

		g_pDlg->m_result.InsertItem(index,IP);

		SetEvent(g_hEvent);

		g_pDlg->m_result.SetItemText(index,1,::g_strPort);

		switch(cmd1[1])

		{

		case 0x02:

			g_pDlg->m_result.SetItemText(index,2,"password");

			break;

		case 0x00:

			g_pDlg->m_result.SetItemText(index,2,"cool");

			break;

		default:

			g_pDlg->m_result.SetItemText(index,2,"unknow");

			break;

		}

	}

	closesocket(sd);

	++::g_Complate;

	--::g_ActiveThread;

	return 0;

以上是一个线程代码~~开启多线程的时候占用较多的CPU,有时候还会出错~~请达人帮忙找一下错误~~

14 个解决方案

#1

这个会存在问题，LZ最好不要在开启线程的时候处理大多的东西，当开启成功后，再说！
如果开启不成功，则启不了相应的效果。
CString IP=((IPData*)lpParam)->IP;需要判断是否转化成功

#2

if(AfxBeginThread()){//相应处理}
这样应该可以判断是否开启成功吧,还有那个转化要怎么判断~~

#3

我看到一个明显的错误是
    ++::g_Complate;
    --::g_ActiveThread;
这2个全局变量的操作没有做互斥保护，应该使用互锁族函数处理
InterlockedIncrement(&g_Complate);
InterlockedDecrement(&g_ActiveThread);
http://msdn2.microsoft.com/en-us/library/ms683614.aspx

#4

自己顶个~~希望洗完澡出来看到更多建议~~

#5

if(AfxBeginThread()){//相应处理}
这样应该可以判断是否开启成功吧,还有那个转化要怎么判断~~

lpParam 如果指向的是局部变量, AfxBeginThread 完后函数返回时该变量就会释放
所以这个 CString IP=((IPData*)lpParam)->IP; 可能就会出错

#6

g_pDlg-> ???
有问题啊

#7

lpParam指向的的确是一个另一个线程的局部变量~不过在所有线程结束之前另一个线程是不会返回的~~不知道这样有没有问题~~
还有g_pDlg不知道有什么问题?这是一个全局指针,指向一个对话框的类~

#8

顶啊~~~

#9

SetItemText实际上调用的是SendMessage, 这会强制同步主线程, 建议改成PostMessage。g_pDlg最好改成对话框句柄, 用PostMessage发送一个自定义消息来实现你的工作。你现在的代码对话框关闭时可能有问题。

#10

在所有线程结束之前我是不让对话框关闭的~~所以我觉得g_pDlg应该是没什么问题的~~

#11

WaitForSingleObject(g_hEvent,INFINITE);有别的线程激活这个g_hEvent吗？要不然这个线程岂不退出去？下面有一句SetEvent(g_hEvent)是不是当前线程由另外一个线程激活，而激活后当前线程去激活另外一个线程？感觉你没有描述清楚，但有可能在这里出现问题： ++::g_Complate;
--::g_ActiveThread;
这2个全局变量的操作没有做互斥保护，应该使用互锁族函数处理

#12

第一对于共享变量的访问请添加互斥机制。
第二对在使用多线程操作请使用_try{} __except(){} SHE进行处理
第三线程的结束请使用线程内安全结束
第四WaitForSingleObject请处理其返回值如：超时，响应，句柄错误等。
第五send,recv不一定每次都能收发到你所需求的长度也许 < 你需求长度也许WSA_IO_PENDING

#13

第六 g_pDlg 可能为CDialog（MFC）那么在您使用该指针的时候请进行安全性检查例如该指针是否为有效指针

#14

第七在closesocket之前Shutdown

#1

#2