Re: [PATCH] Reports: Convert to C++

2021-07-08 Thread Chris Johns
On 9/7/21 3:06 am, Alex White wrote:
> On Wed, Jul 7, 2021 at 6:55 PM Chris Johns  wrote:
>>
>> On 7/7/21 11:37 pm, Ryan Long wrote:
>> > I'll get those pointers changed to references, and remove the whitespace 
>> > changes. Is there a particular reason to not use '\n' instead of std::endl?
>>
>> Awesome and thanks.
>>
>> > I read that std::endl is slower since it's flushing the buffer each time 
>> > that it is used.
>>
>> The std::endl is platform independent so language implementers can match it 
>> to
>> the platform the code is being built for. It is similar to os.linesep in 
>> python
>> and why that should be used there.
> 
> Chris,
> 
> I thought this, too, until Ryan forced me to look into it further. Thanks, 
> Ryan :)
> 
> According to various sources, '\n' gets translated to the current platform's
> line separator as long as the C++ file stream is opened in text mode. See, for
> example, https://stackoverflow.com/a/213977 
> .

Why would you use std::endl in a binary stream? Is the report a binary format?

> So |std::endl|​ would indeed likely be slower AND unnecessary to achieve
> platform independence.

Some platforms require '\r\n' for an end of line. How does this approach handle
that?

Have you measured the performance savings this provides?

I prefer you do not add special code for that sort of stuff and I prefer we
avoid micro-optimisations over following the standards based ways of handling
these things.

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] Reports: Convert to C++

2021-07-08 Thread Alex White

On Wed, Jul 7, 2021 at 6:55 PM Chris Johns  wrote:
>
> On 7/7/21 11:37 pm, Ryan Long wrote:
> > I'll get those pointers changed to references, and remove the whitespace 
> > changes. Is there a particular reason to not use '\n' instead of std::endl?
>
> Awesome and thanks.
>
> > I read that std::endl is slower since it's flushing the buffer each time 
> > that it is used.
>
> The std::endl is platform independent so language implementers can match it to
> the platform the code is being built for. It is similar to os.linesep in 
> python
> and why that should be used there.

Chris,

I thought this, too, until Ryan forced me to look into it further. Thanks, Ryan 
:)

According to various sources, '\n' gets translated to the current platform's 
line separator as long as the C++ file stream is opened in text mode. See, for 
example, https://stackoverflow.com/a/213977.

So std::endl​ would indeed likely be slower AND unnecessary to achieve platform 
independence.

Alex
[https://cdn.sstatic.net/Sites/stackoverflow/Img/apple-touch-i...@2.png?v=73d79a89bded]
"std::endl" vs "\\n"
Many C++ books contain example code like this... std::cout  "Test line" 
 std::endl; ...so I've always done that too. But I've seen a lot of 
code from working developers like this
stackoverflow.com



>
> Chris
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] Reports: Convert to C++

2021-07-07 Thread Chris Johns
On 7/7/21 11:37 pm, Ryan Long wrote:
> I'll get those pointers changed to references, and remove the whitespace 
> changes. Is there a particular reason to not use '\n' instead of std::endl? 

Awesome and thanks.

> I read that std::endl is slower since it's flushing the buffer each time that 
> it is used.

The std::endl is platform independent so language implementers can match it to
the platform the code is being built for. It is similar to os.linesep in python
and why that should be used there.

Chris
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel


RE: [PATCH] Reports: Convert to C++

2021-07-07 Thread Ryan Long
I'll get those pointers changed to references, and remove the whitespace 
changes. Is there a particular reason to not use '\n' instead of std::endl? I 
read that std::endl is slower since it's flushing the buffer each time that it 
is used.

-Original Message-
From: Chris Johns  
Sent: Tuesday, July 6, 2021 7:13 PM
To: Ryan Long ; devel@rtems.org
Subject: Re: [PATCH] Reports: Convert to C++

On 7/7/21 1:53 am, Ryan Long wrote:
> Made formatting consistent and converted C code to C++.
> ---
>  tester/covoar/ReportsBase.cc |  525 +--
>  tester/covoar/ReportsBase.h  |  156 +++---
>  tester/covoar/ReportsHtml.cc | 1183 
> +-
>  tester/covoar/ReportsHtml.h  |  141 ++---
>  tester/covoar/ReportsText.cc |  348 ++---
>  tester/covoar/ReportsText.h  |   75 ++-
>  6 files changed, 1041 insertions(+), 1387 deletions(-)
> 
> diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc
> index 328980d..3041df2 100644
> --- a/tester/covoar/ReportsBase.cc
> +++ b/tester/covoar/ReportsBase.cc
> @@ -4,6 +4,9 @@
>  #include 
>  #include 
>  
> +#include 
> +#include 
> +
>  #include "ReportsBase.h"
>  #include "app_common.h"
>  #include "CoverageRanges.h"
> @@ -20,10 +23,12 @@
>  
>  namespace Coverage {
>  
> -ReportsBase::ReportsBase( time_t timestamp, std::string symbolSetName ):
> -  reportExtension_m(""),
> -  symbolSetName_m(symbolSetName),
> -  timestamp_m( timestamp )
> +ReportsBase::ReportsBase(
> +  time_t timestamp,
> +  const std::string& symbolSetName
> +): reportExtension_m( "" ),
> +   symbolSetName_m( symbolSetName ),
> +   timestamp_m(  timestamp  )
>  {
>  }
>  
> @@ -31,14 +36,14 @@ ReportsBase::~ReportsBase()
>  {
>  }
>  
> -FILE* ReportsBase::OpenFile(
> -  const char* const fileName,
> -  const char* const symbolSetName
> +std::ofstream* ReportsBase::OpenFile(

A raw pointer anywhere is a possible source of a leak and with exceptions it is
hard to track all possible paths.

> +  const std::string& fileName,
> +  const std::string& symbolSetName

Would having ...

 std::ostream& aFile

as a referenced arguement work?

>  )
>  {
> -  int  sc;
> -  FILE*aFile;
> -  std::string  file;
> +  intsc;
> +  std::ofstream* aFile = new std::ofstream;

Please consider std::shared_ptr<> or std::unique_ptr<> for pointers or just
avoid using them which I find simpler.

> +  std::stringfile;
>  
>std::string symbolSetOutputDirectory;
>rld::path::path_join(
> @@ -51,136 +56,110 @@ FILE* ReportsBase::OpenFile(
>  #ifdef _WIN32
>sc = _mkdir( symbolSetOutputDirectory );
>  #else
> -  sc = mkdir( symbolSetOutputDirectory.c_str(),0755 );
> +  sc = mkdir( symbolSetOutputDirectory.c_str(), 0755 );
>  #endif
> -  if ( (sc == -1) && (errno != EEXIST) ) {
> -fprintf(
> -  stderr,
> -  "Unable to create output directory %s\n",
> -  symbolSetOutputDirectory.c_str()
> -);
> +  if ( ( sc == -1 ) && ( errno != EEXIST ) ) {
> +std::cerr << "Unable to create output directory "
> +  << symbolSetOutputDirectory << "\n";

Please do not use "\n", please use std::endl.

Why not throw an exception?

>  return NULL;

NULL should be nullptr in C++ but if this becomes 'void' it can be removed.

>}
>  
>file = symbolSetOutputDirectory;
> -  rld::path::path_join(file, fileName, file);
> +  rld::path::path_join( file, fileName, file );
>  
>// Open the file.
> -  aFile = fopen( file.c_str(), "w" );
> -  if ( !aFile ) {
> -fprintf( stderr, "Unable to open %s\n", file.c_str() );
> +  aFile->open( file );
> +  if ( !aFile->is_open() ) {
> +std::cerr << "Unable to open " << file << "\n";
>}
> +
>return aFile;
>  }
>  
> -void ReportsBase::WriteIndex(
> -  const char* const fileName
> -)
> +void ReportsBase::WriteIndex( const std::string& fileName )
>  {
>  }
>  
> -FILE* ReportsBase::OpenAnnotatedFile(
> -  const char* const fileName
> -)
> +std::ofstream* ReportsBase::OpenAnnotatedFile( const std::string& fileName )

.. and here ..

>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  return OpenFile( fileName, symbolSetName_m );
>  }
>  
> -FILE* ReportsBase::OpenBranchFile(
> -  const char* const fileName,
> -  bool  hasBranches
> +std::ofstream* ReportsBase::OpenBranchFile(

... and here ...

> +  const std::string& fileName,
> + 

Re: [PATCH] Reports: Convert to C++

2021-07-06 Thread Chris Johns
On 7/7/21 1:53 am, Ryan Long wrote:
> Made formatting consistent and converted C code to C++.
> ---
>  tester/covoar/ReportsBase.cc |  525 +--
>  tester/covoar/ReportsBase.h  |  156 +++---
>  tester/covoar/ReportsHtml.cc | 1183 
> +-
>  tester/covoar/ReportsHtml.h  |  141 ++---
>  tester/covoar/ReportsText.cc |  348 ++---
>  tester/covoar/ReportsText.h  |   75 ++-
>  6 files changed, 1041 insertions(+), 1387 deletions(-)
> 
> diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc
> index 328980d..3041df2 100644
> --- a/tester/covoar/ReportsBase.cc
> +++ b/tester/covoar/ReportsBase.cc
> @@ -4,6 +4,9 @@
>  #include 
>  #include 
>  
> +#include 
> +#include 
> +
>  #include "ReportsBase.h"
>  #include "app_common.h"
>  #include "CoverageRanges.h"
> @@ -20,10 +23,12 @@
>  
>  namespace Coverage {
>  
> -ReportsBase::ReportsBase( time_t timestamp, std::string symbolSetName ):
> -  reportExtension_m(""),
> -  symbolSetName_m(symbolSetName),
> -  timestamp_m( timestamp )
> +ReportsBase::ReportsBase(
> +  time_t timestamp,
> +  const std::string& symbolSetName
> +): reportExtension_m( "" ),
> +   symbolSetName_m( symbolSetName ),
> +   timestamp_m(  timestamp  )
>  {
>  }
>  
> @@ -31,14 +36,14 @@ ReportsBase::~ReportsBase()
>  {
>  }
>  
> -FILE* ReportsBase::OpenFile(
> -  const char* const fileName,
> -  const char* const symbolSetName
> +std::ofstream* ReportsBase::OpenFile(

A raw pointer anywhere is a possible source of a leak and with exceptions it is
hard to track all possible paths.

> +  const std::string& fileName,
> +  const std::string& symbolSetName

Would having ...

 std::ostream& aFile

as a referenced arguement work?

>  )
>  {
> -  int  sc;
> -  FILE*aFile;
> -  std::string  file;
> +  intsc;
> +  std::ofstream* aFile = new std::ofstream;

Please consider std::shared_ptr<> or std::unique_ptr<> for pointers or just
avoid using them which I find simpler.

> +  std::stringfile;
>  
>std::string symbolSetOutputDirectory;
>rld::path::path_join(
> @@ -51,136 +56,110 @@ FILE* ReportsBase::OpenFile(
>  #ifdef _WIN32
>sc = _mkdir( symbolSetOutputDirectory );
>  #else
> -  sc = mkdir( symbolSetOutputDirectory.c_str(),0755 );
> +  sc = mkdir( symbolSetOutputDirectory.c_str(), 0755 );
>  #endif
> -  if ( (sc == -1) && (errno != EEXIST) ) {
> -fprintf(
> -  stderr,
> -  "Unable to create output directory %s\n",
> -  symbolSetOutputDirectory.c_str()
> -);
> +  if ( ( sc == -1 ) && ( errno != EEXIST ) ) {
> +std::cerr << "Unable to create output directory "
> +  << symbolSetOutputDirectory << "\n";

Please do not use "\n", please use std::endl.

Why not throw an exception?

>  return NULL;

NULL should be nullptr in C++ but if this becomes 'void' it can be removed.

>}
>  
>file = symbolSetOutputDirectory;
> -  rld::path::path_join(file, fileName, file);
> +  rld::path::path_join( file, fileName, file );
>  
>// Open the file.
> -  aFile = fopen( file.c_str(), "w" );
> -  if ( !aFile ) {
> -fprintf( stderr, "Unable to open %s\n", file.c_str() );
> +  aFile->open( file );
> +  if ( !aFile->is_open() ) {
> +std::cerr << "Unable to open " << file << "\n";
>}
> +
>return aFile;
>  }
>  
> -void ReportsBase::WriteIndex(
> -  const char* const fileName
> -)
> +void ReportsBase::WriteIndex( const std::string& fileName )
>  {
>  }
>  
> -FILE* ReportsBase::OpenAnnotatedFile(
> -  const char* const fileName
> -)
> +std::ofstream* ReportsBase::OpenAnnotatedFile( const std::string& fileName )

.. and here ..

>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  return OpenFile( fileName, symbolSetName_m );
>  }
>  
> -FILE* ReportsBase::OpenBranchFile(
> -  const char* const fileName,
> -  bool  hasBranches
> +std::ofstream* ReportsBase::OpenBranchFile(

... and here ...

> +  const std::string& fileName,
> +  bool   hasBranches
>  )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  return OpenFile( fileName, symbolSetName_m );
>  }
>  
> -FILE* ReportsBase::OpenCoverageFile(
> -  const char* const fileName
> -)
> +std::ofstream* ReportsBase::OpenCoverageFile( const std::string& fileName )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  return OpenFile( fileName, symbolSetName_m );
>  }
>  
> -FILE* ReportsBase::OpenNoRangeFile(
> -  const char* const fileName
> -)
> +std::ofstream* ReportsBase::OpenNoRangeFile( const std::string& fileName )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  return OpenFile( fileName, symbolSetName_m );
>  }
>  
>  
> -FILE* ReportsBase::OpenSizeFile(
> -  const char* const fileName
> -)
> +std::ofstream* ReportsBase::OpenSizeFile( const std::string& fileName )
>  {
> -  return OpenFile(fileName, symbolSetName_m.c_str());
> +  return OpenFile( fileName, symbolSetName_m );
>  }
>  
> -FILE* 

[PATCH] Reports: Convert to C++

2021-07-06 Thread Ryan Long
Made formatting consistent and converted C code to C++.
---
 tester/covoar/ReportsBase.cc |  525 +--
 tester/covoar/ReportsBase.h  |  156 +++---
 tester/covoar/ReportsHtml.cc | 1183 +-
 tester/covoar/ReportsHtml.h  |  141 ++---
 tester/covoar/ReportsText.cc |  348 ++---
 tester/covoar/ReportsText.h  |   75 ++-
 6 files changed, 1041 insertions(+), 1387 deletions(-)

diff --git a/tester/covoar/ReportsBase.cc b/tester/covoar/ReportsBase.cc
index 328980d..3041df2 100644
--- a/tester/covoar/ReportsBase.cc
+++ b/tester/covoar/ReportsBase.cc
@@ -4,6 +4,9 @@
 #include 
 #include 
 
+#include 
+#include 
+
 #include "ReportsBase.h"
 #include "app_common.h"
 #include "CoverageRanges.h"
@@ -20,10 +23,12 @@
 
 namespace Coverage {
 
-ReportsBase::ReportsBase( time_t timestamp, std::string symbolSetName ):
-  reportExtension_m(""),
-  symbolSetName_m(symbolSetName),
-  timestamp_m( timestamp )
+ReportsBase::ReportsBase(
+  time_t timestamp,
+  const std::string& symbolSetName
+): reportExtension_m( "" ),
+   symbolSetName_m( symbolSetName ),
+   timestamp_m(  timestamp  )
 {
 }
 
@@ -31,14 +36,14 @@ ReportsBase::~ReportsBase()
 {
 }
 
-FILE* ReportsBase::OpenFile(
-  const char* const fileName,
-  const char* const symbolSetName
+std::ofstream* ReportsBase::OpenFile(
+  const std::string& fileName,
+  const std::string& symbolSetName
 )
 {
-  int  sc;
-  FILE*aFile;
-  std::string  file;
+  intsc;
+  std::ofstream* aFile = new std::ofstream;
+  std::stringfile;
 
   std::string symbolSetOutputDirectory;
   rld::path::path_join(
@@ -51,136 +56,110 @@ FILE* ReportsBase::OpenFile(
 #ifdef _WIN32
   sc = _mkdir( symbolSetOutputDirectory );
 #else
-  sc = mkdir( symbolSetOutputDirectory.c_str(),0755 );
+  sc = mkdir( symbolSetOutputDirectory.c_str(), 0755 );
 #endif
-  if ( (sc == -1) && (errno != EEXIST) ) {
-fprintf(
-  stderr,
-  "Unable to create output directory %s\n",
-  symbolSetOutputDirectory.c_str()
-);
+  if ( ( sc == -1 ) && ( errno != EEXIST ) ) {
+std::cerr << "Unable to create output directory "
+  << symbolSetOutputDirectory << "\n";
 return NULL;
   }
 
   file = symbolSetOutputDirectory;
-  rld::path::path_join(file, fileName, file);
+  rld::path::path_join( file, fileName, file );
 
   // Open the file.
-  aFile = fopen( file.c_str(), "w" );
-  if ( !aFile ) {
-fprintf( stderr, "Unable to open %s\n", file.c_str() );
+  aFile->open( file );
+  if ( !aFile->is_open() ) {
+std::cerr << "Unable to open " << file << "\n";
   }
+
   return aFile;
 }
 
-void ReportsBase::WriteIndex(
-  const char* const fileName
-)
+void ReportsBase::WriteIndex( const std::string& fileName )
 {
 }
 
-FILE* ReportsBase::OpenAnnotatedFile(
-  const char* const fileName
-)
+std::ofstream* ReportsBase::OpenAnnotatedFile( const std::string& fileName )
 {
-  return OpenFile(fileName, symbolSetName_m.c_str());
+  return OpenFile( fileName, symbolSetName_m );
 }
 
-FILE* ReportsBase::OpenBranchFile(
-  const char* const fileName,
-  bool  hasBranches
+std::ofstream* ReportsBase::OpenBranchFile(
+  const std::string& fileName,
+  bool   hasBranches
 )
 {
-  return OpenFile(fileName, symbolSetName_m.c_str());
+  return OpenFile( fileName, symbolSetName_m );
 }
 
-FILE* ReportsBase::OpenCoverageFile(
-  const char* const fileName
-)
+std::ofstream* ReportsBase::OpenCoverageFile( const std::string& fileName )
 {
-  return OpenFile(fileName, symbolSetName_m.c_str());
+  return OpenFile( fileName, symbolSetName_m );
 }
 
-FILE* ReportsBase::OpenNoRangeFile(
-  const char* const fileName
-)
+std::ofstream* ReportsBase::OpenNoRangeFile( const std::string& fileName )
 {
-  return OpenFile(fileName, symbolSetName_m.c_str());
+  return OpenFile( fileName, symbolSetName_m );
 }
 
 
-FILE* ReportsBase::OpenSizeFile(
-  const char* const fileName
-)
+std::ofstream* ReportsBase::OpenSizeFile( const std::string& fileName )
 {
-  return OpenFile(fileName, symbolSetName_m.c_str());
+  return OpenFile( fileName, symbolSetName_m );
 }
 
-FILE* ReportsBase::OpenSymbolSummaryFile(
-  const char* const fileName
+std::ofstream* ReportsBase::OpenSymbolSummaryFile(
+  const std::string& fileName
 )
 {
-  return OpenFile(fileName, symbolSetName_m.c_str());
+  return OpenFile( fileName, symbolSetName_m );
 }
 
-void ReportsBase::CloseFile(
-  FILE*  aFile
-)
+void ReportsBase::CloseFile( std::ofstream* aFile )
 {
-  fclose( aFile );
+  aFile->close();
 }
 
-void ReportsBase::CloseAnnotatedFile(
-  FILE*  aFile
-)
+void ReportsBase::CloseAnnotatedFile( std::ofstream* aFile )
 {
   CloseFile( aFile );
 }
 
-void ReportsBase::CloseBranchFile(
-  FILE*  aFile,
-  bool   hasBranches
-)
+void ReportsBase::CloseBranchFile( std::ofstream* aFile, bool hasBranches )
 {
   CloseFile( aFile );
 }
 
-void  ReportsBase::CloseCoverageFile(
-  FILE*  aFile
-)
+void  ReportsBase::CloseCoverageFile(