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

[Cillian O'Donnell]

It does seem to be having some knock on effect. Covoar made it past these
checks before.

-----------------
Total:         13
Average test time: 0:00:03.178923
Testing time     : 0:00:41.326000
Running covoar for score
covoar results directory:
/home/cpod/development/rtems/leon3/coverage/score
ERROR: executable build prefix does not match: sparc-rtems5
***Cleaning tempfiles***
error: covoar failure exit code: 1

Not sure how thats related.

Its checking

 if (buildPrefix.empty()) {
 76           buildPrefix =
*pri;
 77         } else
{
 78           if (buildPrefix != *pri)
{
 79             std::cout << "buildBSP: " + buildBSP << "\n*pri: " + *pri
<< std::en
    dl;
 80             fail = "executable build prefix does not match: " +
buildPrefix;
 81
break;
 82
}
 83         }

I added those checks, Its stepping back through the path and checking if
each directory makes sense. It seems to be out of line now

ERROR: executable build prefix does not match: sparc-rtems5
buildBSP: leon3
*pri: sparc-rtems5
***Cleaning tempfiles***

Maybe its splitting the path on '.' and '/' Haven't checked it yet.

On 13 May 2018 at 07:05, Vijay Kumar Banerjee <vijaykumar9597 at gmail.com>
wrote:

>
> On 13 May 2018 at 10:50, Chris Johns <chrisj at rtems.org> wrote:
>
>> 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?
>>
>> 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 .
>
> Do we need the .exe extension there ?
>
> > 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?
>>
>
> I can look into this. Any instructions ?
>
>>
>> Chris
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20180513/fce87fa9/attachment-0001.html>