String::Format_VA memory leaks #17405

Broxzier · 2022-06-18T18:11:09Z

Operating System

Windows 10, 64 bit

OpenRCT2 build

OpenRCT2, v0.4.0-174-gc8df66f (c8df66f on develop) provided by GitHub

Describe the issue

The implementation of String::Format_VA (mostly called from the String::Format overload without the buffer argument) allocates space for the string, and it's up to the user to free this (Memory::Free). This is however rarely done, instead it is mostly used as an argument for assignment to a std::string.

OpenRCT2/src/openrct2/core/String.cpp

Lines 355 to 406 in 13db5a7

    
           utf8* Format_VA(const utf8* format, va_list args) 
        
           { 
        
               va_list args1, args2; 
        
               va_copy(args1, args); 
        
               va_copy(args2, args); 
        
               // Try to format to a initial buffer, enlarge if not big enough 
        
               size_t bufferSize = 4096; 
        
               utf8* buffer = Memory::Allocate<utf8>(bufferSize); 
        
               // Start with initial buffer 
        
               int32_t len = vsnprintf(buffer, bufferSize, format, args); 
        
               if (len < 0) 
        
               { 
        
                   Memory::Free(buffer); 
        
                   va_end(args1); 
        
                   va_end(args2); 
        
                   // An error occurred... 
        
                   return nullptr; 
        
               } 
        
               size_t requiredSize = static_cast<size_t>(len) + 1; 
        
               if (requiredSize > bufferSize) 
        
               { 
        
                   // Try again with bigger buffer 
        
                   buffer = Memory::Reallocate<utf8>(buffer, bufferSize); 
        
                   len = vsnprintf(buffer, bufferSize, format, args); 
        
                   if (len < 0) 
        
                   { 
        
                       Memory::Free(buffer); 
        
                       va_end(args1); 
        
                       va_end(args2); 
        
                       // An error occurred... 
        
                       return nullptr; 
        
                   } 
        
               } 
        
               else 
        
               { 
        
                   // Reduce buffer size to only what was required 
        
                   bufferSize = requiredSize; 
        
                   buffer = Memory::Reallocate<utf8>(buffer, bufferSize); 
        
               } 
        
               // Ensure buffer is terminated 
        
               buffer[bufferSize - 1] = '\0'; 
        
               va_end(args1); 
        
               va_end(args2); 
        
               return buffer; 
        
           }

Since almost all cases use std::string already, the best approach would be to return a std::string or a u8string here.

Area(s) with issue?

Building the game

The text was updated successfully, but these errors were encountered:

SaumyaBhushan · 2022-06-19T07:39:47Z

I would like to take this.

Broxzier added bug good first issue refactor labels Jun 18, 2022

String::Format_VA memory leaks #17405

String::Format_VA memory leaks #17405

Broxzier commented Jun 18, 2022 •

edited

SaumyaBhushan commented Jun 19, 2022

String::Format_VA memory leaks #17405

String::Format_VA memory leaks #17405

Comments

Broxzier commented Jun 18, 2022 • edited

Operating System

OpenRCT2 build

Describe the issue

Area(s) with issue?

SaumyaBhushan commented Jun 19, 2022

Broxzier commented Jun 18, 2022 •

edited