金山卫士源代码达不到工业标准

金山卫士源代码达不到工业 金山卫士开源代码显然达不到工业级的标准 - 秋窗会 - Autumn Window Net [A.W.N]分享到... 当前位置:新闻 ? 业界动态 ? 新闻内容 >> 金山卫士开源代码显然达不到工业级的标准 金山卫士开源代码显然达不到工业级的标准更新时间:2010-12-21 12:51:21 作者:佚名 来源:本站整理 这几天忙着看有关C的书籍,无意中看见某牛人写的对金山开源代码的批评,汗，大家一起看看吧, ------------------------------------------------------------------------------------------------------------ -------------------------------------------- 金山卫士开源了，参见金山卫士开源。 抱着学习研究的目的下了一份看看。看了一些代码，觉得被忽悠了。中国知名通用软件厂商，民族软件业的一面旗帜就这代码水平?代码显然达不到工业级的标准，只能算是实习生练手的水准。为了给有意拿这份代码当学习资料的初学者提个醒，不被误导，做出了一个艰难的决定，写博文来评论金山安全卫士的代码。 先说说代码中的几个突出问 C++应用不过关的‎‎。该用const和static的时候不用 代码封装做的不好，调用者知道被调用者很多细节，且对被调用者做了过多假设。 文件和函数命名不。不能表达内容，且容易引起误解 测试靠打印而不是assert,很难自动化验证。且测试代码未与代码分离。 太多的if-else而不会用表驱动 函数逻辑不严格，有明显漏洞。 一点一点的看 1 C++的应用不过关。该用const和static的时候不用 ppro\PrivacyProtection\rule\IRule.h class IRule { public: // MichaelPeng: Name函数可以设置为const virtual LPCTSTR Name(void) = 0; virtual void Enable(BOOL bEnable){} // MichaelPeng: Match从语义上也应为const,且参数pData也应为const virtual BOOL Match(KAccessData* pData) = 0; // MichaelPeng: DbgPrint从语义上也应为const virtual void DbgPrint(void) = 0; }; 2 代码封装做的不好，调用者知道被调用者很多细节，且对被调用者做了过多假 设。 ppro\PrivacyProtection\rule\KDirRelateRule.cpp BOOL KDirRelateRule::Match(KAccessData* pData) { BOOL bRetCode = FALSE; iter; std::map>::iterator for (iter = m_mapDirRelates.begin(); iter != m_mapDirRelates.end(); iter++) { const CString& strDir = iter->first; // MicahelPeng: 在向m_mapDirRelated中插入数据时都调用MakeLower转化成了小写，但在比较时 却不转化，假定调用者作了转化?? if (strDir.GetLength() <= pData->nFileDirLen && memcmp((LPCTSTR)strDir, pData->szFilePath, strDir.GetLength() * sizeof(TCHAR)) == 0) { std::vector::iterator iterRelate; std::vector& vecStr = iter->second; for (iterRelate = vecStr.begin(); iterRelate != vecStr.end(); ++iterRelate) { int nMemSize = pData->nProcPathLen * sizeof(TCHAR); CString& strTemp = *iterRelate; if (strTemp.GetLength() == pData->nProcPathLen && memcmp((LPCTSTR)strTemp, pData->szProcPath, nMemSize) == 0) { return TRUE; } } } } // MichaelPeng:szProcPath应当类似于C:\windows\notepad.exe, 这里需要保证nProcPathLen和nProcDirLen都被正确设置, // 最好是KAccessData提供方法SetProcPath,在其中将szProcPath, nProcPathLen, nProcDirLen均设置了 // 但没有这种函数，需要靠调用者去保证每次都将这三个变量同时正确设置j int nProcNameLen = pData->nProcPathLen - pData->nProcDirLen; LPCTSTR pProcStr = pData->szProcPath + pData->nProcDirLen; 没有必要声明两个iterator // MicaelPeng: 遍历的容器都一样, std::map>::iterator iter2; // ... } 3 文件和函数命名不规划。不能表达内容，且容易引起误解 RuleManager.cpp，你看到这个文件名第一反应这个文件 是做什么用的?管理rule的是吧，接下来看到的代码会超越你所有的常识。 // KRuleManager.cpp : Defines the entry point for the console application. // #include "stdafx.h" #include "KFileExtensionRule.h" #include "KProcFileRule.h" #include "KFileDirRule.h" #include "KFilePathRule.h" #include "KFileExtRelateRule.h" #include "KFileNameRelateRule.h" #include "KDirRelateRule.h" #include "KProcPathRule.h" #include "KSignDataRule.h" #include "KVariableString.h" #include "KRuleConfig.h" #include "KTree.h" #include "KRuleImpl.h" #include "KRuleTestImpl.h" #include "signverify.h" #include "KSignCache.h" #include "KProcNameCache.h" void TestFileExtensionRule(KAccessData* pData) { KFileExtensionRule rule; rule.AddFileExt(_T(".jpg")); rule.AddFileExt(_T(".html")); rule.AddFileExt(_T(".dll")); rule.AddFileExt(_T(".html")); rule.AddFileExt(_T(".DLL")); rule.RemoveFileExt(_T(".dll")); BOOL bRetCode = rule.Match(pData); // MichaelPeng: 通过打印而非Assert来校验测试结果,不可重复和自动化 DPrintA("KFileExtensionRule::Match return:%d\n", bRetCode); } void TestTree(void) { KTree tree; tree.SetValue(0); tree.AddLeft(1); tree.AddRight(2); // MichaelPeng: 这里测了啥,没有抛异常就OK了??? } void TestRule(void) { ....... // MichaelPeng: 这么多KAccessData设置的雷同代码，为何不放到一个数组中 用表驱动实现 { KAccessData data; // MichaelPeng: 拷贝字符串和设置长度可放到KAccessData的成员函数中 一次完成 // 代码冗余,暴露给外界太多信息,且这里也未设置nProdDirLen,与BOOL KDirRelateRule::Match(KAccessData* pData)的要求矛盾 _tcscpy(data.szProcPath, _T("d:\\a\\b.exe")); _tcscpy(data.szFilePath, _T("c:\\a\\b\\b.doc")); data.nProcPathLen= _tcslen(data.szProcPath); data.nFilePathLen = _tcslen(data.szFilePath); TestKDirRelateRule(&data); } { KAccessData data; _ [1] [2] [3] 下一页 当前位置:新闻 ? 业界动态 ? 新闻内容 >> 金山卫士开源代码显然达不到工业级 的标准 金山卫士开源代码显然达不到工业级的标准更新时间:2010-12-21 12:51:21 作 者:佚名 来源:本站整理 ProcPath, _T("c:\\a\\b\\e.exe")); tcscpy(data.sz _tcscpy(data.szFilePath, _T("c:\\a\\b\\b.doc")); data.nProcPathLen= _tcslen(data.szProcPath); data.nFilePathLen = _tcslen(data.szFilePath); TestKProcPathRule(&data); } { KAccessData data; _tcscpy(data.szProcPath, _T("c:\\WINDOWS\\system32\\notepad.exe")); _tcscpy(data.szFilePath, _T("c:\\a\\b\\b.doc")); data.nProcPathLen= _tcslen(data.szProcPath); data.nFilePathLen = _tcslen(data.szFilePath); TestKSignDataRule(&data); } { KAccessData data; _tcscpy(data.szProcPath, _T("c:\\Program Files\\Common Files\\Kingsoft\\kiscommon\\kisuisp.exe")); _tcscpy(data.szFilePath, _T("c:\\a\\b\\b.doc")); data.nProcPathLen= _tcslen(data.szProcPath); data.nFilePathLen = _tcslen(data.szFilePath); TestKSignDataRule(&data); } TestKVariableString(); TestCreateRules(); TestTree(); } int _tmain(int argc, _TCHAR* argv[]) { CSignVerify::Instance().Initialize(); //TestRule(); //TestRuleImpl(); //TestRuleImplMultiThread(); //TestRuleTestImpl(); //TestSign(); //TestSignCacheAndProcNameCache(); //TestUserConfig(); // MichaelPeng: 测试代码未与工程代码清晰分离 TestLoadRule(); return 0; } 4 测试靠打印而不是assert,很难自动化验证。且测试代码未与工程代码分离。 见上例 5 太多的if-else而不会用表驱动 ppro\PrivacyProtection\rule\KSystemEnvirVar.h class KSystemEnvirVar { public: // MichaelPeng: 应为静态函数 CString GetValue(LPCTSTR szVariable) { TCHAR szFolderPath[MAX_PATH + 1] = {0}; // MichaelPeng: if else太多,应做成表驱动 if (0 == _tcsicmp(szVariable, _T("%Desktop%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_DESKTOP, 0); else } if (0 == _tcsicmp(szVariable, _T("%Internet%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_INTERNET, 0); } else if (0 == _tcsicmp(szVariable, _T("%Programs%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_PROGRAMS, 0); } else if (0 == _tcsicmp(szVariable, _T("%Controls%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_CONTROLS, 0); } else if (0 == _tcsicmp(szVariable, _T("%Printers%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_PRINTERS, 0); } else if (0 == _tcsicmp(szVariable, _T("%Personal%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_PERSONAL, 0); } else if (0 == _tcsicmp(szVariable, _T("%Favorites%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_FAVORITES, 0); } else if (0 == _tcsicmp(szVariable, _T("%Startup%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_STARTUP, 0); } else if (0 == _tcsicmp(szVariable, _T("%Recent%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_RECENT, 0); } else if (0 == _tcsicmp(szVariable, _T("%Sendto%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_SENDTO, 0); } else if (0 == _tcsicmp(szVariable, _T("%Bitbucket%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_BITBUCKET, 0); } else if (0 == _tcsicmp(szVariable, _T("%StartMenu%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_STARTMENU, 0); } else if (0 == _tcsicmp(szVariable, _T("%Mydocuments%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_MYDOCUMENTS, 0); } else if (0 == _tcsicmp(szVariable, _T("%Mymusic%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_MYMUSIC, 0); } else if (0 == _tcsicmp(szVariable, _T("%Myvideo%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_MYVIDEO, 0); } else if (0 == _tcsicmp(szVariable, _T("%Desktopdirectory%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_DESKTOPDIRECTORY, 0); } else if (0 == _tcsicmp(szVariable, _T("%Drives%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_DRIVES, 0); } else if (0 == _tcsicmp(szVariable, _T("%Network%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_NETWORK, 0); } else if (0 == _tcsicmp(szVariable, _T("%Nethood%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_NETHOOD, 0); } else if (0 == _tcsicmp(szVariable, _T("%Fonts%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_FONTS, 0); } else if (0 == _tcsicmp(szVariable, _T("%Templates%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_TEMPLATES, 0); } else if (0 == _tcsicmp(szVariable, _T("%CommonStartMenu%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_COMMON_STARTMENU, 0); } else if (0 == _tcsicmp(szVariable, _T("%CommonPrograms%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_COMMON_PROGRAMS, 0); } else if (0 == _tcsicmp(szVariable, _T("%CommonStartup%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_COMMON_STARTUP, 0); } else if (0 == _tcsicmp(szVariable, _T("%LocalAppdate%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_LOCAL_APPDATA, 0); else } if (0 == _tcsicmp(szVariable, _T("%CommonAppdata%"))) { ::S 上一页 [1] [2] [3] 下一页 当前位置:新闻 ? 业界动态 ? 新闻内容 >> 金山卫士开源代码显然达不到工业级的标准 金山卫士开源代码显然达不到工业级的标准更新时间:2010-12-21 12:51:21 作 者:佚名 来源:本站整理 HGetSpecialFolderPath(NULL, szFolderPath, CSIDL_COMMON_APPDATA, 0); } else if (0 == _tcsicmp(szVariable, _T("%Windows%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_WINDOWS, 0); } else if (0 == _tcsicmp(szVariable, _T("%System32%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_SYSTEM, 0); } else if (0 == _tcsicmp(szVariable, _T("%ProgramFilesComm%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_PROGRAM_FILES_COMMON, 0); } else if (0 == _tcsicmp(szVariable, _T("%CommonDocuments%"))) { ::SHGetSpecialFolderPath(NULL, szFolderPath, CSIDL_COMMON_DOCUMENTS, 0); } else { CString strVariable(szVariable); strVariable.Remove(_T('%')); TCHAR* szTemp = _tgetenv(strVariable); if (szTemp) { if (_tcsstr(szTemp, L"~")) { ::GetLongPathNameW(szTemp, szFolderPath, MAX_PATH); } else { _tcsncpy(szFolderPath, szTemp, MAX_PATH); } szFolderPath[MAX_PATH] = 0; } } return szFolderPath; } protected: }; 6 函数逻辑不严格，有明显漏洞 ppro\PrivacyProtection\rule\KVariableString.h BOOL KVariableString::RelpaceVariable(CString& strString) { BOOL bReturn = TRUE; int nStartPos; int nEndPos; CString strTemp(strString); CString strVariable; CString strValue; for (;;) { nStartPos = strTemp.Find(_T('%'), 0); if (nStartPos != -1) nEndPos = strTemp.Find(_T('%'), nStartPos + 1); if (nStartPos == -1 || nEndPos == -1) break; strVariable = strTemp.Mid(nStartPos, nEndPos - nStartPos + 1); strValue = GetVariable(strVariable); if (strValue.IsEmpty()) break; // MichaelPeng: 对于%abc%xxx%def%abc%ghi%,会出现错误的替换,正确的是替 换掉%abc%, %def%, %ghi%,但这段代码会替换掉两个%abc% strTemp.Replace(strVariable, strValue); } if (strTemp.Find(_T('%')) != -1) { #ifdef OUTPUT_INIT_WARNING #ifdef __log_file__ CString strTempVar(strString); strTempVar.Replace(_T("%"), _T("%%")); DPrintW(L"KVariableString::RelpaceVariable fail, variablename:%s\n", strTempVar); CString strMsg; strMsg.Format(_T("查找环境变量失败:%s"), strString); ::MessageBox(NULL, strMsg, _T("隐私保护器"), MB_OK); #endif #endif return FALSE; } // MicaelPeng: 这个替换功能不能从函数名体现出来 do { nStartPos = strTemp.Find(_T("\\\\")); if (nStartPos == -1) break; strTemp.Replace(_T("\\\\"), _T("\\")); } while (true); strString = strTemp; \rule\KFunction.cpp ppro\PrivacyProtection // MichaelPeng: DirCount是什么意思?路径数目?DirLength更合适 int KFunction::GetDirCount(LPCTSTR szPath) { LPCTSTR pszChr = _tcsrchr(szPath, _T('\\')); if (!pszChr) return -1; return pszChr - szPath + 1; } // MicahelPeng: count应为length int KFunction::GetFileNameCount(LPCTSTR szPath) { LPCTSTR pszChr = _tcsrchr(szPath, _T('\\')); if (!pszChr) return -1; return _tcslen(++pszChr); } // MichaelPeng: count应为length // 若文件没有后缀而路径有后缀，如C:\a.b.c\dfgh 则结果应为0或-1，但函数返回 7 int KFunction::GetFileExtCount(LPCTSTR szPath) { LPCTSTR pszChr = _tcsrchr(szPath, _T('.')); if (!pszChr) return -1; return _tcslen(pszChr); } 上一页 [1] [2] [3]