Re: [LyX/master] Fix typo found by coverity

2017-03-14 Thread Richard Heck 
On 03/14/2017 09:28 AM, Jean-Marc Lasgouttes wrote:
> Le 14/03/2017 à 04:41, Richard Heck a écrit :
>> On 03/13/2017 12:05 PM, Jean-Marc Lasgouttes wrote:
>>> Le 13/03/2017 à 16:18, Richard Heck a écrit :
 I'll move the formats variable to the singleton, too. That seems worth
 doing, yes, and should be painless?
>>>
>>> I think it should be a bit long, but painless.
>>
>> Done. Not too long, but definitely painless.
>
> I fixed some problems found in the commit (2 warnings and a filaed test).

Thanks for catching the warnings, which were obviously genuine problems.
I'm not sure why I did not get them.

Nice illustration of why the global 'formats' variable was a bad idea.

Richard

Re: [LyX/master] Fix typo found by coverity

2017-03-14 Thread Jean-Marc Lasgouttes 

Le 14/03/2017 à 04:41, Richard Heck a écrit :

On 03/13/2017 12:05 PM, Jean-Marc Lasgouttes wrote:

Le 13/03/2017 à 16:18, Richard Heck a écrit :

I'll move the formats variable to the singleton, too. That seems worth
doing, yes, and should be painless?


I think it should be a bit long, but painless.


Done. Not too long, but definitely painless.


I fixed some problems found in the commit (2 warnings and a filaed test).

JMarc

Re: [LyX/master] Fix typo found by coverity

2017-03-14 Thread Jean-Marc Lasgouttes 

Le 14/03/2017 à 04:41, Richard Heck a écrit :

Done. Not too long, but definitely painless.


Thanks.

JMarc

Re: [LyX/master] Fix typo found by coverity

2017-03-13 Thread Richard Heck 
On 03/13/2017 12:05 PM, Jean-Marc Lasgouttes wrote:
> Le 13/03/2017 à 16:18, Richard Heck a écrit :
>> I'll move the formats variable to the singleton, too. That seems worth
>> doing, yes, and should be painless?
>
> I think it should be a bit long, but painless.

Done. Not too long, but definitely painless.

rh

Re: [LyX/master] Fix typo found by coverity

2017-03-13 Thread Jean-Marc Lasgouttes 

Le 13/03/2017 à 16:18, Richard Heck a écrit :

I'll move the formats variable to the singleton, too. That seems worth
doing, yes, and should be painless?


I think it should be a bit long, but painless.

JMarc

Re: [LyX/master] Fix typo found by coverity

2017-03-13 Thread Richard Heck 
On 03/13/2017 06:43 AM, Jean-Marc Lasgouttes wrote:
> Le 07/03/2017 à 21:14, Richard Heck a écrit :
>>> Actually there are other uses of the variable `formats' in
>>> Formats.cpp, and I think that they are all incorrect (should be
>>> formatlist). This will not matter in most cases, fortunately.
>>
>> I will try to have a look at this.
>
> I did that at a2bfe0042d. Please double check.

Will do.

I'll move the formats variable to the singleton, too. That seems worth
doing, yes, and should be painless?

Richard

Re: [LyX/master] Fix typo found by coverity

2017-03-13 Thread Jean-Marc Lasgouttes 

Le 07/03/2017 à 21:14, Richard Heck a écrit :

Actually there are other uses of the variable `formats' in
Formats.cpp, and I think that they are all incorrect (should be
formatlist). This will not matter in most cases, fortunately.


I will try to have a look at this.


I did that at a2bfe0042d. Please double check.

JMarc

Re: [LyX/master] Fix typo found by coverity

2017-03-07 Thread Richard Heck 
On 03/07/2017 06:16 AM, Jean-Marc Lasgouttes wrote:
> Le 07/03/2017 à 12:08, Jean-Marc Lasgouttes a écrit :
>> commit dc126bad0441eb8721043e597657d5c18bdf6c90
>> Author: Jean-Marc Lasgouttes 
>> Date:   Tue Mar 7 12:02:54 2017 +0100
>>
>> Fix typo found by coverity
>>
>> We were not testing for the right end(), although it is not sure
>> that
>> an actual bug could be triggered because of that.
>
> Richard,
>
> This is a bug that you marked as false positive on coverity. Do you
> agree now that the fix was needed?

Yes, that looks right.

> Actually there are other uses of the variable `formats' in
> Formats.cpp, and I think that they are all incorrect (should be
> formatlist). This will not matter in most cases, fortunately.

I will try to have a look at this.

> A good way to avoid this would be to instantiate formats and
> system_formats in some other .cpp file. They should not be available
> in Format.cpp. What is the correct idiom for this?

I'm afraid I would not know that

Richard

Re: [LyX/master] Fix typo found by coverity

2017-03-07 Thread Jean-Marc Lasgouttes 

Le 07/03/2017 à 12:08, Jean-Marc Lasgouttes a écrit :

commit dc126bad0441eb8721043e597657d5c18bdf6c90
Author: Jean-Marc Lasgouttes 
Date:   Tue Mar 7 12:02:54 2017 +0100

Fix typo found by coverity

We were not testing for the right end(), although it is not sure that
an actual bug could be triggered because of that.


Richard,

This is a bug that you marked as false positive on coverity. Do you 
agree now that the fix was needed?


Actually there are other uses of the variable `formats' in Formats.cpp, 
and I think that they are all incorrect (should be formatlist). This 
will not matter in most cases, fortunately.


A good way to avoid this would be to instantiate formats and 
system_formats in some other .cpp file. They should not be available in 
Format.cpp. What is the correct idiom for this?


JMarc


---
 src/Format.cpp |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/Format.cpp b/src/Format.cpp
index b605c4e..1b53d73 100644
--- a/src/Format.cpp
+++ b/src/Format.cpp
@@ -433,7 +433,7 @@ string Formats::getFormatFromFile(FileName const & 
filename) const
Formats::const_iterator cit =
find_if(formatlist.begin(), 
formatlist.end(),
FormatMimeEqual(mime));
-   if (cit != formats.end()) {
+   if (cit != formatlist.end()) {
LYXERR(Debug::GRAPHICS, "\tgot 
format from MIME type: "
<< mime << " -> " << 
cit->name());
// See special eps/ps handling 
below