<div dir="ltr"><div dir="ltr">On Tue, Mar 2, 2021 at 5:37 PM Chris Johns <<a href="mailto:chrisj@rtems.org">chrisj@rtems.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2/3/21 7:01 am, Alex White wrote:<br>
> This adds a new macro USE_TEMPLFILE which allows the creation of named<br>
> objdump outputs in the /tmp directory. This allows the outputs to be<br>
> reused on subsequent runs of covoar rather than running objdump again.<br>
<br>
Is the USE_TEMPLFILE macro a debugging aid?<br>
<br>
If it is and it is useful could it be a command line argument to select a mode<br>
that allows debugging? It means the code is always seen by the compiler and<br>
built and anyone needing to chase down an issue has it available without needing<br>
the code and rebuilding it. Plus as an argument the command will have some help<br>
and that can be the documentation for this option.<br></blockquote><div><br></div><div>Yes, it is both a debugging aid and a way to speed up execution.</div><div><br></div><div>I agree that it would be better as a command line argument. I will change it and add it to the appropriate patch set.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> ---<br>
> rtemstoolkit/rld-process.cpp | 12 ++++++++++++<br>
> rtemstoolkit/rld-process.h | 9 +++++++++<br>
<br>
<br>
> tester/covoar/ObjdumpProcessor.cc | 16 +++++++++++-----<br>
> tester/covoar/app_common.cc | 10 ++++++++++<br>
> tester/covoar/app_common.h | 1 +<br>
> tester/covoar/covoar.cc | 20 ++++++++++++++++++++<br>
> 6 files changed, 63 insertions(+), 5 deletions(-)<br>
> <br>
> diff --git a/rtemstoolkit/rld-process.cpp b/rtemstoolkit/rld-process.cpp<br>
> index 30e0605..d0352cb 100644<br>
> --- a/rtemstoolkit/rld-process.cpp<br>
> +++ b/rtemstoolkit/rld-process.cpp<br>
> @@ -169,6 +169,18 @@ namespace rld<br>
> _name = temporaries.get (suffix, _keep);<br>
> }<br>
> <br>
> + tempfile::tempfile (<br>
> + const std::string& name,<br>
> + const std::string& suffix,<br>
> + bool _keep<br>
> + ) : _name(name + suffix),<br>
> + suffix(suffix),<br>
> + overridden (false),<br>
> + fd (-1),<br>
> + level (0)<br>
> + {<br>
> + }<br>
> +<br>
> tempfile::~tempfile ()<br>
> {<br>
> try<br>
> diff --git a/rtemstoolkit/rld-process.h b/rtemstoolkit/rld-process.h<br>
> index fc9b7bc..16e0322 100644<br>
> --- a/rtemstoolkit/rld-process.h<br>
> +++ b/rtemstoolkit/rld-process.h<br>
> @@ -114,6 +114,15 @@ namespace rld<br>
> */<br>
> tempfile (const std::string& suffix = ".rldxx", bool keep = false);<br>
> <br>
> + /**<br>
> + * Get a temporary file name given a name and a suffix.<br>
> + */<br>
> + tempfile (<br>
> + const std::string& name,<br>
> + const std::string& suffix,<br>
> + bool _keep = false<br>
> + );<br>
> +<br>
<br>
The addition is fine but there is a minor nit, the coding format I used in this<br>
code from long ago is:<br>
<br>
tempfile (const std::string& name,<br>
const std::string& suffix,<br>
bool _keep = false);<br>
<br>
Would it be possible to keep it consistent with the other code, eg see `output`<br>
further down in this file?<br>
<br>
Should this also be a separate patch?<br>
<br>
Chris<br></blockquote><div><br></div><div>I will fix this for my RLD patch set.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> /**<br>
> * Clean up the temporary file.<br>
> */<br>
> diff --git a/tester/covoar/ObjdumpProcessor.cc b/tester/covoar/ObjdumpProcessor.cc<br>
> index 00824f1..f130819 100644<br>
> --- a/tester/covoar/ObjdumpProcessor.cc<br>
> +++ b/tester/covoar/ObjdumpProcessor.cc<br>
> @@ -245,12 +245,18 @@ namespace Coverage {<br>
> fileName };<br>
> try<br>
> {<br>
> - status = rld::process::execute( TargetInfo->getObjdump(),<br>
> - args, objdumpFile.name(), <a href="http://err.name" rel="noreferrer" target="_blank">err.name</a>() );<br>
> - if ( (status.type != rld::process::status::normal)<br>
> - || (status.code != 0) ) {<br>
> - throw rld::error( "Objdump error", "generating objdump" );<br>
> + #ifndef USE_TEMPFILE<br>
> + if (FileIsNewer( fileName.c_str(), objdumpFile.name().c_str() )) {<br>
> + #endif<br>
> + status = rld::process::execute( TargetInfo->getObjdump(),<br>
> + args, objdumpFile.name(), <a href="http://err.name" rel="noreferrer" target="_blank">err.name</a>() );<br>
> + if ( (status.type != rld::process::status::normal)<br>
> + || (status.code != 0) ) {<br>
> + throw rld::error( "Objdump error", "generating objdump" );<br>
> + }<br>
> + #ifndef USE_TEMPFILE<br>
> }<br>
> + #endif<br>
> } catch( rld::error& err )<br>
> {<br>
> std::cout << "Error while running " << TargetInfo->getObjdump()<br>
> diff --git a/tester/covoar/app_common.cc b/tester/covoar/app_common.cc<br>
> index 8b490ed..0f3c8e2 100644<br>
> --- a/tester/covoar/app_common.cc<br>
> +++ b/tester/covoar/app_common.cc<br>
> @@ -116,3 +116,13 @@ bool ReadUntilFound( FILE *file, const char *line )<br>
> } while (1);<br>
> }<br>
> <br>
> +std::string GetFileNameFromPath( const std::string& path )<br>
> +{<br>
> + size_t idx = path.rfind('/', path.length());<br>
> + if (idx == std::string::npos) {<br>
> + return "";<br>
> + }<br>
> +<br>
> + return path.substr(idx + 1, path.length() - idx);<br>
> +}<br>
> +<br>
> diff --git a/tester/covoar/app_common.h b/tester/covoar/app_common.h<br>
> index ac32bbd..21e8cfa 100644<br>
> --- a/tester/covoar/app_common.h<br>
> +++ b/tester/covoar/app_common.h<br>
> @@ -30,5 +30,6 @@ extern char inputBuffer2[MAX_LINE_LENGTH];<br>
> bool FileIsNewer( const char *f1, const char *f2 );<br>
> bool FileIsReadable( const char *f1 );<br>
> bool ReadUntilFound( FILE *file, const char *line );<br>
> +std::string GetFileNameFromPath( const std::string& path );<br>
> <br>
> #endif<br>
> diff --git a/tester/covoar/covoar.cc b/tester/covoar/covoar.cc<br>
> index bf95cb4..bc1b7cf 100644<br>
> --- a/tester/covoar/covoar.cc<br>
> +++ b/tester/covoar/covoar.cc<br>
> @@ -162,7 +162,9 @@ int covoar(<br>
> FILE* gcnosFile = NULL;<br>
> Gcov::GcovData* gcovFile;<br>
> const char* singleExecutable = NULL;<br>
> + #ifdef USE_TEMPFILE<br>
> rld::process::tempfile objdumpFile( ".dmp" );<br>
> + #endif<br>
> rld::process::tempfile err( ".err" );<br>
> rld::process::tempfile syms( ".syms" );<br>
> bool debug = false;<br>
> @@ -373,6 +375,22 @@ int covoar(<br>
> exe->setLoadAddress( objdumpProcessor->determineLoadAddress( exe ) );<br>
> }<br>
> <br>
> + #ifndef USE_TEMPFILE<br>
> + std::string name;<br>
> + <br>
> + if ( !exe->hasDynamicLibrary() ) {<br>
> + name = exe->getFileName();<br>
> + } else {<br>
> + name = exe->getLibraryName();<br>
> + }<br>
> +<br>
> + name = GetFileNameFromPath( name );<br>
> + name = buildTarget + "-" + name;<br>
> + name.insert( 0, "/tmp/" );<br>
> +<br>
> + rld::process::tempfile objdumpFile( name, ".dmp", true );<br>
> + #endif<br>
> +<br>
> // Load the objdump for the symbols in this executable.<br>
> objdumpProcessor->load( exe, objdumpFile, err );<br>
> }<br>
> @@ -486,8 +504,10 @@ int covoar(<br>
> <br>
> //Leave tempfiles around if debug flag (-d) is enabled.<br>
> if ( debug ) {<br>
> + #ifdef USE_TEMPFILE<br>
> objdumpFile.override( "objdump_file" );<br>
> objdumpFile.keep();<br>
> + #endif<br>
> err.override( "objdump_exec_log" );<br>
> err.keep();<br>
> syms.override( "symbols_list" );<br>
> <br>
</blockquote></div></div>