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

[Vijay Kumar Banerjee]

On Sun, 13 May 2018, 16:09 Cillian O'Donnell, <cpodonnell8 at gmail.com> wrote:

> 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***
>
initially there were two problems
+exe.cov and .cov mismatch
+access library from outside the leon3 directory.

The patch solves the first one, that's one step .
The next step can be solved by adding the path to the build-target , i.e.
sparc-rtems5 , from HOME directory, you can manually add it and see it
running for now. That tells us that inclusion of the path in more standard
way will solve it .

>
> 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/207f31ce/attachment-0002.html>