[PATCH] covoar: fixing the extension mismatch in trace file

[Chris Johns]

On 13/5/18 4:44 pm, Vijay Kumar Banerjee wrote:
> On Sun, 13 May 2018, 10:09 Chris Johns, <chrisj at rtems.org
> <mailto:chrisj at rtems.org>> wrote:
>     >
>     >        Coverage::CoverageFormats_t   coverageFormat =
>     >     Coverage::COVERAGE_FORMAT_QEMU;
>     >        Coverage::CoverageReaderBase* coverageReader = NULL;
>     >        char*                         executable = NULL;
>     >     @@ -317,11 +317,12 @@ int main(
>     >              std::cerr << "warning: Unable to read executable: " << argv[i] <<
>     >     std::endl;
>     >            } else {
>     >              coverageFileName = argv[i];
>     >     -        coverageFileName.replace(
>     >     +       coverageFileName.append(coverageExtension);
>     >     +     /* coverageFileName.replace(
>     >                coverageFileName.length() - executableExtension.size(),
>     >                executableExtension.size(),
>     >                coverageExtension
>     >     -        );
>     >     +        ); */
>     >
>     >
>     >  
>     > If you are replacing this call, then just delete it -- don't comment it out.
>     >
> 
>     I suggest the replace be changed to move past the '.' in the file name.
> 
>     I suspect the code here is still fragile because the extensions need to be the
>     same size.
> 
> can't this be done by adding '.' in the append as in the v2 of this patch to
> keep the extension size same  ?

It fixes something but what is broken that is being fixed?

> are you suggesting to use replace instead ?

I think it would be better to fix the bug than work around it.

The replace is saying replace from length of the extension back from the end of
the string for the size of the extension with the extension. In other words it
should back up from the end the extension length and replace it. Which length or
size is wrong.

No matter what the current code is fragile because it assumes a few things. The
most robust approach would add code to the rtemstoolkit's rld::path namespace to
return a path with the extension stripped. For example add
'rld::path::strip_extension ()' and then use that to strip coverageFileName and
then add the extension.

I would also review the 'exe' extension usage to make sure it is OK. I have not
done this.

Are you interested in doing this?

Chris