<div dir="ltr"><br><div class="gmail_extra"><div class="gmail_quote">On 13 May 2018 at 10:50, Chris Johns <span dir="ltr"><<a href="mailto:chrisj@rtems.org" target="_blank">chrisj@rtems.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 13/5/18 4:44 pm, Vijay Kumar Banerjee wrote:<br>
> On Sun, 13 May 2018, 10:09 Chris Johns, <<a href="mailto:chrisj@rtems.org">chrisj@rtems.org</a><br>
</span><div><div class="h5">> <mailto:<a href="mailto:chrisj@rtems.org">chrisj@rtems.org</a>>> wrote:<br>
>     ><br>
>     >        Coverage::CoverageFormats_t   coverageFormat =<br>
>     >     Coverage::COVERAGE_FORMAT_<wbr>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(<wbr>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>
> <br>
> can't this be done by adding '.' in the append as in the v2 of this patch to<br>
> keep the extension size same  ?<br>
<br>
</div></div>It fixes something but what is broken that is being fixed?<br>
<span class=""><br></span></blockquote><div>the trace files have .exe.cov extensions but the covoar was searching for .cov extension causing it to return error for not finding the trace file. Adding the .cov to the coverageFileName to make it .exe.cov seemed like a quick fix to this .</div><div><br></div><div>Do we need the .exe extension there ?  </div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
> are you suggesting to use replace instead ?<br>
<br>
</span>I think it would be better to fix the bug than work around it.<br>
<br>
The replace is saying replace from length of the extension back from the end of<br>
the string for the size of the extension with the extension. In other words it<br>
should back up from the end the extension length and replace it. Which length or<br>
size is wrong.<br>
<br>
No matter what the current code is fragile because it assumes a few things. The<br>
most robust approach would add code to the rtemstoolkit's rld::path namespace to<br>
return a path with the extension stripped. For example add<br>
'rld::path::strip_extension ()' and then use that to strip coverageFileName and<br>
then add the extension.<br>
<br>
I would also review the 'exe' extension usage to make sure it is OK. I have not<br>
done this.<br>
<br>
Are you interested in doing this?<br></blockquote><div> </div><div>I can look into this. Any instructions ?  </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
Chris<br>
</font></span></blockquote></div><br></div></div>