[PATCH v1 1/6] Explanations.cc: Fix resource leaks

[Ryan Long]

We'll submit the C++ conversions as separate patches since they are larger. That patch will be submitted shortly.

-----Original Message-----
From: Chris Johns <chrisj at rtems.org> 
Sent: Friday, May 28, 2021 8:05 PM
To: Ryan Long <ryan.long at oarcorp.com>; devel at rtems.org
Subject: Re: [PATCH v1 1/6] Explanations.cc: Fix resource leaks

On 29/5/21 8:08 am, Ryan Long wrote:
> CID 1399608: Resource leak in load().
> CID 1399611: Resource leak in load().
> 
> Closes #4417
> ---
>  tester/covoar/Explanations.cc | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/tester/covoar/Explanations.cc 
> b/tester/covoar/Explanations.cc index 12bd809..d94cd2e 100644
> --- a/tester/covoar/Explanations.cc
> +++ b/tester/covoar/Explanations.cc
> @@ -32,7 +32,7 @@ namespace Coverage {
>      #define MAX_LINE_LENGTH 512
>      FILE       *explain;
>      char        *cStatus;
> -    Explanation *e;
> +    Explanation  e;
>      int          line = 1;
>  
>      if (!explanations)
> @@ -46,7 +46,6 @@ namespace Coverage {
>      }
>  
>      while ( 1 ) {
> -      e = new Explanation;
>        // Read the starting line of this explanation and
>        // skip blank lines between
>        do {
> @@ -65,12 +64,13 @@ namespace Coverage {
>          what << "line " << line
>               << "contains a duplicate explanation ("
>               << inputBuffer << ")";
> +        fclose( explain );
>          throw rld::error( what, "Explanations::load" );

If you used a std::ostream type object it would be automatically closed when that object destructs via any exit path.

While this patch fixes the bug(s) the mixing of C here is generating these issues and effort cleaning that up will reward you.

Chris

>        }
>  
>        // Add the starting line and file
> -      e->startingPoint = std::string(inputBuffer);
> -      e->found = false;
> +      e.startingPoint = std::string(inputBuffer);
> +      e.found = false;
>  
>        // Get the classification
>        cStatus = fgets( inputBuffer, MAX_LINE_LENGTH, explain ); @@ 
> -78,10 +78,11 @@ namespace Coverage {
>          std::ostringstream what;
>          what << "line " << line
>               << "out of sync at the classification";
> +        fclose( explain );
>          throw rld::error( what, "Explanations::load" );
>        }
>        inputBuffer[ strlen(inputBuffer) - 1] = '\0';
> -      e->classification = inputBuffer;
> +      e.classification = inputBuffer;
>        line++;
>  
>        // Get the explanation
> @@ -92,6 +93,7 @@ namespace Coverage {
>            std::ostringstream what;
>            what << "line " << line
>                 << "out of sync at the explanation";
> +          fclose( explain );
>            throw rld::error( what, "Explanations::load" );
>          }
>          inputBuffer[ strlen(inputBuffer) - 1] = '\0'; @@ -102,15 
> +104,17 @@ namespace Coverage {
>            break;
>          }
>          // XXX only taking last line.  Needs to be a vector
> -        e->explanation.push_back( inputBuffer );
> +        e.explanation.push_back( inputBuffer );
>        }
>  
>        // Add this to the set of Explanations
> -      set[ e->startingPoint ] = *e;
> +      set[ e.startingPoint ] = e;
>      }
>  done:
>      ;
>  
> +    fclose( explain );
> +
>      #undef MAX_LINE_LENGTH
>    }
>  
>