स्रोत एसडीके परियोजना विश्लेषण

स्रोत एसडीके - वाल्व निगम द्वारा विकसित, स्रोत इंजन पर संशोधन बनाने के लिए उपयोगिताओं का एक सेट। परियोजना के स्रोत कोड 2013 के अंत में डाउनलोड और सत्यापित किए गए थे। नए साल की छुट्टियों पर, मैंने निरीक्षण के परिणामों के बारे में एक लेख लिखने की योजना बनाई। लेकिन आलस्य रचनात्मकता पर जीत गया, और मैंने केवल एक लेख लिखना शुरू किया जब मैं काम पर लौट आया। हालांकि, मुझे लगता है कि यह संभावना नहीं है कि इस अवधि के दौरान कुछ स्रोत कोड में परिवर्तन करने में कामयाब रहे। मेरा सुझाव है कि आप अपने आप को उन संदिग्ध स्थानों से परिचित करें जो मैंने पीवीएस-स्टूडियो कोड विश्लेषक का उपयोग करके पाया था।

स्रोत एसडीके क्या है

विकिपीडिया से एक परियोजना विवरण उधार लें:

स्रोत एसडीके (सॉफ्टवेयर डेवलपमेंट किट) सोर्स इंजन पर संशोधन बनाने के लिए उपयोगिताओं का एक सेट है, जो वाल्व उन सभी खिलाड़ियों को स्टीम नेटवर्क के माध्यम से मुफ्त में वितरित करता है जो वाल्व से किसी भी स्रोत गेम को खरीदते हैं। यह सेट आपको इंजन के दो संस्करणों पर मैप्स को संपादित करने की अनुमति देता है - 15 वीं और अपडेट किए गए 7 वें (हाफ-लाइफ 2 में उपयोग किए गए इंजन का पुराना संस्करण नए संस्करण के साथ संगतता के कारण उपयोग नहीं किया गया है)।

प्रोजेक्ट साइट: http://source.valvesoftware.com/sourcedk.php

परियोजना काफी बड़ी है, इसलिए यह आश्चर्यजनक नहीं है कि इसमें कमियों की पहचान करना हमेशा संभव होता है। पीवीएस-स्टूडियो टूल का उपयोग करके विश्लेषण किया गया था।

संदेहास्पद भाव

अपने आप में परिवर्तनशील चर

static void DrawPyroVignette(....) { .... Vector2D vMaxSize( ( float )nScreenWidth / ( float )nScreenWidth / NUM_PYRO_SEGMENTS * 2.0f, ( float )nScreenHeight / ( float )nScreenHeight / NUM_PYRO_SEGMENTS * 2.0f ); .... } 

PVS- स्टूडियो निम्नलिखित फ़ाइल के लिए एक चेतावनी V501 जारी करता है: viewpostprocess.cpp 1888

इस पर ध्यान दें:

• (फ्लोट) nScreenWidth / (फ्लोट) nScreenWidth
• (फ्लोट) nScreenHeight / (फ्लोट) nScreenHeight

ये बहुत ही संदिग्ध भाव हैं। मुझे यह कहना मुश्किल है कि यहां क्या लिखा जाना चाहिए, लेकिन सबसे अधिक संभावना है, कुछ और।

IsJoystickPOVCode () फ़ंक्शन के लिए डबल कॉल

 void TextEntry::OnKeyCodePressed(KeyCode code) { .... if ( IsMouseCode(code) || IsNovintButtonCode(code) || IsJoystickCode(code) || IsJoystickButtonCode(code) || IsJoystickPOVCode(code) || IsJoystickPOVCode(code) || IsJoystickAxisCode(code) ) .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक चेतावनी V501 जारी करता है: textentry.cpp 1639

'IsJoystickPOVCode (कोड)' फ़ंक्शन को दो बार कहा जाता है। दूसरा कॉल निरर्थक है या किसी अन्य फ़ंक्शन को कॉल किया जाना चाहिए था।

हमेशा एक झूठी स्थिति

 unsigned numbounce = 100; int ParseCommandLine( int argc, char **argv, bool *onlydetail ) { .... numbounce = atoi (argv[i]); if ( numbounce < 0 ) { Warning( "Error: expected non-negative value after '-bounce'\n"); return 1; } .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V547 चेतावनी जारी करता है: vrad.cpp 2412।

शर्त "सुन्न <0" कभी पूरी नहीं होगी। एक अहस्ताक्षरित चर शून्य से कम नहीं हो सकता।

अजीब स्ट्रिंग तुलना

 void CMultiplayRules::DeathNotice( .... ) { .... else if ( strncmp( killer_weapon_name, "NPC_", 8 ) == 0 ) .... } 

पीवीएस-स्टूडियो निम्नलिखित फ़ाइल के लिए एक V666 चेतावनी जारी करता है: मल्टीप्ले_गैमरल्स।सीपी 860।

जैसा कि मैं इसे समझता हूं, वे यह सत्यापित करना चाहते थे कि हथियार का नाम "एनपीसी_" अक्षर से शुरू होता है। लेकिन फिर इस कोड में एक टाइपो होता है। संभवतः सही विकल्प यह है:

 else if ( strncmp( killer_weapon_name, "NPC_", 4 ) == 0 ) 

सरणियों के साथ काम करते समय त्रुटियां

गलत सरणी आकार

 #define RTL_NUMBER_OF_V1(A) (sizeof(A)/sizeof((A)[0])) #define _ARRAYSIZE(A) RTL_NUMBER_OF_V1(A) int GetAllNeighbors( const CCoreDispInfo *pDisp, int iNeighbors[512] ) { .... if ( nNeighbors < _ARRAYSIZE( iNeighbors ) ) iNeighbors[nNeighbors++] = pCorner->m_Neighbors[i]; .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V511 चेतावनी जारी करता है: disp_vrad.cpp 60

औपचारिक तर्क "int iNeighbors [512]" एक सरणी नहीं है। यह सिर्फ एक संकेतक है। संख्या '512' प्रोग्रामर को बताती है कि सबसे अधिक संभावना है कि सूचक 512 तत्वों की एक सरणी को संदर्भित करता है। लेकिन इससे ज्यादा नहीं। अभिव्यक्ति 'साइज़ोफ़ (iNeighbors)' अवैध है। यह सरणी का आकार नहीं लौटाता है, लेकिन सूचक का आकार। अर्थात्, अभिव्यक्ति 'sizeof (iNeighbors)' 'sizeof (void *) के बराबर होगी।

अधिक सुरक्षित मैक्रो का उपयोग करके इस त्रुटि से बचा जा सकता था। उदाहरण के लिए, यह:

 template <typename T, size_t N> char (&ArraySizeHelper(T (&array)[N]))[N]; #define arraysize(array) (sizeof(ArraySizeHelper(array))) 

सूचक के आकार की गणना करने की कोशिश करते समय, संकलन चरण में एक त्रुटि होती है। क्रोमियम प्रोजेक्ट में इस तरह के मैक्रो का उपयोग किया जाता है। " पीवीएस-स्टूडियो बनाम क्रोमियम " लेख में इस जादू के निर्माण के बारे में और पढ़ें।

स्ट्रिंग की लंबाई की गलत परिभाषा

 typedef struct message_s { .... char *text; .... } message_t; int CMessageCharsPanel::AddText(....) { .... msg->text = new char[ Q_strlen( data ) + 1 ]; Assert( msg->text ); Q_strncpy( msg->text, data, sizeof( msg->text ) ); .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक चेतावनी V579 जारी करता है: vgui_messagechars.cpp 240

अभिव्यक्ति "sizeof (संदेश-> पाठ)" सूचक के आकार की गणना करता है, न कि स्ट्रिंग की लंबाई। सबसे अधिक संभावना है, आपको यहां लिखना चाहिए: Q_strcpy (संदेश-> पाठ, डेटा);

नष्ट सरणी के साथ काम करें

 static Activity DetermineExpressionMoveActivity( CChoreoEvent *event, CAI_BaseNPC *pNPC ) { .... const char *pszAct = Q_strstr( sParam2, " " ); if ( pszAct ) { char szActName[256]; Q_strncpy( szActName, sParam2, sizeof(szActName) ); szActName[ (pszAct-sParam2) ] = '\0'; pszAct = szActName; } .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक चेतावनी V507 जारी करता है: baseflex.cpp 1326

अस्थायी सरणी का पता चर 'pszAct' में रखा गया है। चूंकि यह सरणी नष्ट हो जाएगी, इसलिए आप 'pszAct' में मौजूद पते का उपयोग नहीं कर सकते। हालांकि, व्यवहार में, यह कोड काम कर सकता है, जो कोड की शुद्धता की गलत उपस्थिति बनाता है। यह बहुत संभावना है कि अस्थायी सरणी 'szActName' द्वारा कब्जा किए गए मेमोरी क्षेत्र का आगे उपयोग नहीं किया जाता है। परिणामस्वरूप, प्रोग्रामर अपेक्षा करता है कि प्रोग्राम व्यवहार करता है। हालांकि, यह भाग्य है और अधिक कुछ नहीं है।

एक सरणी में विदेश जा रहे हैं

 #define MAX_WEAPON_SLOTS 6 // hud item selection slots void CHudWeaponSelection::Paint() { .... int xModifiers[] = { 0, 1, 0, -1 }; int yModifiers[] = { -1, 0, 1, 0 }; for ( int i = 0; i < MAX_WEAPON_SLOTS; ++i ) { .... xPos += ( m_flMediumBoxWide + 5 ) * xModifiers[ i ]; yPos += ( m_flMediumBoxTall + 5 ) * yModifiers[ i ]; .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक चेतावनी V557 जारी करता है : hud_weaponselection.cpp 632, 633।

लूप काउंटर 0 से 6 तक मान लेता है। हालाँकि, xModifiers और yModifiers सरणियों में केवल 4 तत्व होते हैं। नतीजतन, सरणी का एक अतिप्रवाह होगा।

नए ऑपरेटर का खतरनाक उपयोग।

अर्थहीन जाँच

 void EmitDispLMAlphaAndNeighbors() { .... CCoreDispInfo *pDisp = new CCoreDispInfo; if ( !pDisp ) { g_CoreDispInfos.Purge(); return; } .... } 

PVS- स्टूडियो निम्नलिखित फ़ाइल के लिए एक चेतावनी V668 जारी करता है: disp_vbsp.cpp 532।

यदि आप 'CCoreDispInfo' प्रकार का ऑब्जेक्ट नहीं बना सकते हैं, तो g_CoreDispInfos.Purge () फ़ंक्शन को कॉल करना होगा। लेकिन यह फ़ंक्शन नहीं कहा जाएगा। यदि स्मृति आबंटन त्रुटि होती है, तो एक अपवाद std :: bad_alloc फेंका जाएगा। इस कोड को हटा दिया गया है और इसे 'नए' ऑपरेटर के व्यवहार में परिवर्तन को प्रतिबिंबित करने के लिए संशोधित करने की आवश्यकता है।

अन्य स्थानों पर जहां यह जाँच की जाती है कि 'नया' ऑपरेटर लौटाया गया है, परिशिष्ट में लेख के अंत में दिया जाएगा।

विध्वंसक में नया संचालक

 CNewParticleEffect::~CNewParticleEffect(void) { .... KeyValues *msg = new KeyValues( "ParticleSystem_Destroy" ); .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V509 चेतावनी जारी करता है: कण_new.cpp 92।

विध्वंसक में निर्माणों का उपयोग करना खतरनाक है जो अपवाद का कारण बन सकता है। यह ठीक 'नए' संचालक के निर्माण का है। मेमोरी आवंटन त्रुटि की स्थिति में, यह एक अपवाद फेंकता है।

मैं खतरे को समझाऊंगा। यदि प्रोग्राम में कोई अपवाद होता है, तो स्टैक फोल्डिंग शुरू होती है, जिसके दौरान ऑब्जेक्ट्स को डिस्ट्रक्टर्स कहकर नष्ट किया जाता है। यदि स्टैक के ढहने पर ऑब्जेक्ट का डिस्ट्रॉयर नष्ट हो जाता है और एक अन्य अपवाद फेंकता है, और विध्वंसक इस अपवाद को छोड़ देता है, तो C ++ लाइब्रेरी तुरंत टर्मिनेट () फ़ंक्शन को कॉल करके प्रोग्राम को क्रैश कर देती है।

शुद्धिपत्र

नेस्टेड लूप में त्रुटि

 void DrawTeslaSegs(....) { int i; .... for ( i = 0; i < segments; i++ ) { .... for ( int j = 0; i < iBranches; j++ ) { curSeg.m_flWidth *= 0.5; } .... } .... } 

PVS-Studio एक V534 को निम्न फ़ाइल के लिए चेतावनी जारी करता है: beadraw.cpp 592।

दूसरे चक्र पर ध्यान दें:

 for ( int j = 0; i < iBranches; j++ ) 

नेस्टेड लूप को पूरा करने की स्थिति में, चर 'i' का उपयोग किया जाता है, जो बाहरी लूप को संदर्भित करता है। मुझे एक मजबूत संदेह है कि हम एक टाइपो के साथ काम कर रहे हैं।

गलत शुरुआत

 inline void SetX( float val ); inline void SetY( float val ); inline void SetZ( float val ); inline void SetW( float val ); inline void Init( float ix=0, float iy=0, float iz=0, float iw = 0 ) { SetX( ix ); SetY( iy ); SetZ( iz ); SetZ( iw ); } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V525 चेतावनी जारी करता है: networkvar.h 455।

मुझे ऐसा लगता है कि फ़ंक्शन को इस तरह लिखा जाना चाहिए था:

 { SetX( ix ); SetY( iy ); SetZ( iz ); SetW( iw ); } 

अंतिम फ़ंक्शन कॉल पर ध्यान दें।

कॉपी-पेस्ट का नतीजा

 class ALIGN16 FourVectors { public: fltx4 x, y, z; .... }; FourVectors BackgroundColor; void RayTracingEnvironment::RenderScene(....) { .... intens.x=OrSIMD(AndSIMD(BackgroundColor.x,no_hit_mask), AndNotSIMD(no_hit_mask,intens.x)); intens.y=OrSIMD(AndSIMD(BackgroundColor.y,no_hit_mask), AndNotSIMD(no_hit_mask,intens.y)); intens.z=OrSIMD(AndSIMD(BackgroundColor.y,no_hit_mask), AndNotSIMD(no_hit_mask,intens.z)); .... } 

पीवीएस-स्टूडियो निम्नलिखित फ़ाइल के लिए एक चेतावनी V537 जारी करता है: ट्रेस2.cpp 189।

सबसे अधिक संभावना है, यह कोड कॉपी-पेस्ट का उपयोग करके लिखा गया था। देखें, पहली पंक्ति 'x' नाम के वर्ग के सदस्यों का उपयोग करती है। दूसरे में, 'y' नाम के साथ। और तीसरे में 'z' और 'y' दोनों है। सबसे अधिक संभावना है, कोड इस तरह होना चाहिए:

 intens.z=OrSIMD(AndSIMD(BackgroundColor.z,no_hit_mask), AndNotSIMD(no_hit_mask,intens.z)); 

एक चर के लिए अलग-अलग मान निर्दिष्ट करना

 void GetFPSColor( int nFps, unsigned char ucColor[3] ) { .... int nFPSThreshold1 = 20; int nFPSThreshold2 = 15; if (IsPC() && g_pMaterialSystemHardwareConfig->GetDXSupportLevel() >= 95) { nFPSThreshold1 = 60; nFPSThreshold1 = 50; } .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V519 चेतावनी जारी करता है: vgui_fpspanel.cpp 192।

जाहिर है, यह लिखा जाना चाहिए था:

 nFPSThreshold1 = 60; nFPSThreshold2 = 50; 

बुरा निर्माण करनेवाला

 CAI_ShotRegulator::CAI_ShotRegulator() : m_nMinBurstShots(1), m_nMaxBurstShots(1) { m_flMinRestInterval = 0.0f; m_flMinRestInterval = 0.0f; m_flMinBurstInterval = 0.0f; m_flMaxBurstInterval = 0.0f; m_flNextShotTime = -1; m_nBurstShotsRemaining = 1; m_bInRestInterval = false; m_bDisabled = false; } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V519 चेतावनी जारी करता है: ai_utils.cpp 49।

फिर से एक टाइपो, जिसके कारण:

1. चर m_flMinRestInterval दो बार शून्य पर सेट किया गया है।
2. वैरिएबल m_flMaxRestInterval अनइंस्टॉल किया गया है।

क्लास कंस्ट्रक्टर्स CEnvTonemapController, CBasePlayerAnimState में इसी तरह की परेशानी है। लेकिन समान उदाहरणों का वर्णन करना उबाऊ है, इसलिए मैंने संबंधित लोगों को आवेदन में डाल दिया (लेख का अंत देखें)।

अपरिभाषित व्यवहार

यौगिक भाव

 int m_nNewSequenceParity; int m_nResetEventsParity; void C_BaseAnimating::ResetSequenceInfo( void ) { .... m_nNewSequenceParity = ( ++m_nNewSequenceParity ) & EF_PARITY_MASK; m_nResetEventsParity = ( ++m_nResetEventsParity ) & EF_PARITY_MASK; .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक चेतावनी V567 जारी करता है: c_baseanimating.cpp 5301, 5302।

अपरिभाषित व्यवहार यहाँ क्यों होता है और चर का मान 'm_nResetEventsParity' क्यों नहीं हो सकता है, इस बात की भविष्यवाणी की जा सकती है कि इसे प्रलेखन में अच्छी तरह से वर्णित किया गया है। विवरण में बहुत समान कोड का एक उदाहरण है।

बदलाव

 inline void SetStyleType( int w, int h, int type ) { Assert( type < NUM_EDGE_STYLES ); Assert( type >= 0 ); // Clear old value m_nPanelBits[ w ][ h ] &= ( ~0x03 << 2 ); // Insert new value m_nPanelBits[ w ][ h ] |= ( type << 2 ); } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V610 चेतावनी जारी करता है: c_func_breakablesurf.cpp 157।

नकारात्मक संख्याओं को शिफ्ट करने से अपरिभाषित व्यवहार होता है। इस कोड में, नकारात्मक संख्या '~ 0x03' है। अधिक विस्तार से, नकारात्मक संख्याओं को स्थानांतरित करने के मुद्दे पर मैंने पहले ही लेख में विचार किया है " कांटा नहीं जानता, पानी में नहीं। भाग तीन ।"

वर्चुअल विध्वंसक गुम है

 class CFlashlightEffect { .... ~CFlashlightEffect(); .... }; class CHeadlightEffect : public CFlashlightEffect { .... }; CFlashlightEffect *m_pFlashlight; C_BasePlayer::~C_BasePlayer() { .... delete m_pFlashlight; } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V599 चेतावनी जारी करता है: c_baseplayer.cpp 454।

एक वर्ग CFlashlightEffect है। इसका कोई वर्चुअल विध्वंसक नहीं है। हालाँकि, CHeadlightEffect वर्ग इस वर्ग से विरासत में मिला है। इससे उत्पन्न परिणाम, मुझे लगता है, समझ में आता है।

संदेहास्पद अंकगणित

परियोजना में बहुत सारे स्थान हैं जहां पूर्णांक प्रकार और फ्लोटिंग बिंदु प्रकार अजीब तरह से संयुक्त हैं। यह संदेह है कि कुछ गणनाएं पर्याप्त सटीक नहीं होंगी या बिल्कुल भी समझ में नहीं आएंगी। मैं केवल 3 उदाहरणों के नीचे प्रदर्शित करूंगा। शेष संदिग्ध स्थानों को ऐप में सूचीबद्ध किया गया है।

पहला संदिग्ध टुकड़ा

 void TE_BloodStream(....) { .... int speedCopy = amount; .... speedCopy -= 0.00001; // so last few will drip .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V674 चेतावनी जारी करता है: c_te_bloodstream.cpp 141।

0.00001 को 'int' के एक वैरिएबल से घटाया जाना बहुत संदेहास्पद है।

दूसरा संदिग्ध टुकड़ा

 #define ON_EPSILON 0.1 void CPrediction::SetIdealPitch (....) { int step; .... step = floor_height[j] - floor_height[j-1]; if (step > -ON_EPSILON && step < ON_EPSILON) continue; .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक चेतावनी V674 जारी करता है: prediction.cpp 977।

चर प्रकार 'चरण' बहुत अच्छी तरह से चयनित नहीं है।

तीसरा संदिग्ध टुकड़ा

 virtual int GetMappingWidth( ) = 0; virtual int GetMappingHeight( ) = 0; void CDetailObjectSystem::LevelInitPreEntity() { .... float flRatio = pMat->GetMappingWidth() / pMat->GetMappingHeight(); .... } 

PVS-Studio निम्नलिखित फ़ाइल के लिए एक V636 चेतावनी जारी करता है: detailobjectsystem.cpp 1480।

शायद यह चर 'flRatio' के मूल्य की अधिक सटीक गणना करने के लिए समझ में आता है। कम सटीकता का कारण पूर्णांक विभाजन है। सटीकता बढ़ाने के लिए, आप इसे लिख सकते हैं:

 float flRatio = static_cast<float>(pMat->GetMappingWidth()) / pMat->GetMappingHeight(); 

अनेक वस्तुओं का संग्रह

प्रकार भ्रम

 enum PhysGunPickup_t { PICKED_UP_BY_CANNON, PUNTED_BY_CANNON, PICKED_UP_BY_PLAYER, }; enum PhysGunDrop_t { DROPPED_BY_PLAYER, THROWN_BY_PLAYER, DROPPED_BY_CANNON, LAUNCHED_BY_CANNON, }; void CBreakableProp::OnPhysGunDrop(...., PhysGunDrop_t Reason) { .... if( Reason == PUNTED_BY_CANNON ) { PlayPuntSound(); } .... } 

PVS- स्टूडियो निम्नलिखित फ़ाइल के लिए एक चेतावनी V556 जारी करता है: 1520 props.cpp।

चर 'कारण' PhysGunDrop_t प्रकार का है। और 'PUNTED_BY_CANNON' 'PhysGunPickup_t' को संदर्भित करता है।

संभावित खतरनाक fprintf

 static void Exit(const char *msg) { fprintf( stderr, msg ); Pause(); exit( -1 ); } 

पीवीएस-स्टूडियो निम्नलिखित फ़ाइल के लिए एक V618 चेतावनी जारी करता है: वाइस 52 पीपी।

'Fprintf ()' फंक्शन काफी सही ढंग से काम कर सकता है। हालांकि, यह संभावित रूप से खतरनाक है। यदि नियंत्रण वर्ण गलती से या जानबूझकर स्ट्रिंग 'msg' में निहित हैं, तो परिणाम अप्रत्याशित होंगे।

इस विषय पर एक दिलचस्प टिप्पणी: " बिना कांटे को जाने, पानी में नहीं उतरें। भाग दो ।"

कोड का सुरक्षित संस्करण:

 fprintf( stderr, "%s", msg ); 

आवेदन

अन्य पीवीएस-स्टूडियो चेतावनियाँ, जो मेरी राय में, ध्यान देने योग्य हैं, फ़ाइल में सूचीबद्ध होंगी। लेकिन आपको इस सूची पर पूरी तरह से भरोसा नहीं करना चाहिए। मैंने परियोजना को सतही रूप से देखा और अधिक ध्यान नहीं दे सका। साथ ही, स्थैतिक विश्लेषण का वास्तविक लाभ इसका नियमित उपयोग है, न कि परियोजना का एकमुश्त सत्यापन।

अन्य संदिग्ध स्थानों की सूची: source-sdk-addition-log.txt

निष्कर्ष

मुझे उम्मीद है कि यह लेख स्रोत एसडीके परियोजना के डेवलपर्स के लिए पाठकों के लिए दिलचस्प और उपयोगी था।