<div dir="auto"><div><br><div class="gmail_quote"><div dir="ltr">On Sun, 13 May 2018, 10:09 Chris Johns, <<a href="mailto:chrisj@rtems.org">chrisj@rtems.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Sorry for the post in the other thread. I did not see this one.<br>
<br>
On 13/5/18 9:28 am, Joel Sherrill wrote:<br>
> <br>
> <br>
> On Sat, May 12, 2018 at 2:42 PM, thelunatic <<a href="mailto:vijaykumar9597@gmail.com" target="_blank" rel="noreferrer">vijaykumar9597@gmail.com</a><br>
> <mailto:<a href="mailto:vijaykumar9597@gmail.com" target="_blank" rel="noreferrer">vijaykumar9597@gmail.com</a>>> wrote:<br>
> <br>
>     ---<br>
> <br>
> <br>
> Every patch needs an explanation/log message.  Look back through <br>
> "git log" for examples.<br>
>  <br>
> <br>
>      tester/covoar/covoar.cc | 7 ++++---<br>
>      1 file changed, 4 insertions(+), 3 deletions(-)<br>
> <br>
>     diff --git a/tester/covoar/covoar.cc b/tester/covoar/covoar.cc<br>
>     index 5bed98f..95dc990 100644<br>
>     --- a/tester/covoar/covoar.cc<br>
>     +++ b/tester/covoar/covoar.cc<br>
>     @@ -185,7 +185,7 @@ int main(<br>
>        Executables                   executablesToAnalyze;<br>
>        Coverage::ExecutableInfo*     executableInfo = NULL;<br>
>        std::string                   executableExtension = "exe";<br>
>     -  std::string                   coverageExtension = "cov";<br>
>     +  std::string                   coverageExtension = ".cov";<br>
> <br>
> <br>
> Why did this need a "." but the "exe" one didn't? <br>
>  <br>
<br>
Yes I was wondering this and why I made the change to the coverage extension.<br>
<br>
> <br>
>        Coverage::CoverageFormats_t   coverageFormat =<br>
>     Coverage::COVERAGE_FORMAT_QEMU;<br>
>        Coverage::CoverageReaderBase* coverageReader = NULL;<br>
>        char*                         executable = NULL;<br>
>     @@ -317,11 +317,12 @@ int main(<br>
>              std::cerr << "warning: Unable to read executable: " << argv[i] <<<br>
>     std::endl;<br>
>            } else {<br>
>              coverageFileName = argv[i];<br>
>     -        coverageFileName.replace(<br>
>     +       coverageFileName.append(coverageExtension);<br>
>     +     /* coverageFileName.replace(<br>
>                coverageFileName.length() - executableExtension.size(),<br>
>                executableExtension.size(),<br>
>                coverageExtension<br>
>     -        );<br>
>     +        ); */<br>
> <br>
> <br>
>  <br>
> If you are replacing this call, then just delete it -- don't comment it out.<br>
> <br>
<br>
I suggest the replace be changed to move past the '.' in the file name.<br>
<br>
I suspect the code here is still fragile because the extensions need to be the<br>
same size.<br></blockquote></div></div><div dir="auto">can't this be done by adding '.' in the append as in the v2 of this patch to keep the extension size same  ?</div><div dir="auto">are you suggesting to use replace instead ?</div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Chris<br>
<br>
> <br>
>              if (!FileIsReadable( coverageFileName.c_str() )) {<br>
>                std::cerr << "warning: Unable to read coverage file: " <<<br>
>     coverageFileName<br>
>     -- <br>
>     2.14.3<br>
> <br>
> <br>
</blockquote></div></div></div>