Bug #970

Coverity scan : Issue CID 981992 , 981993

Added by Mahesh Hegde almost 3 years ago. Updated almost 2 years ago.

Status:ClosedStart date:19 Jul 2014
Priority:NormalDue date:31 Aug 2014
Assignee:Niels Kristian Bech Jensen% Done:

100%

Category:metadata
Target version:0.25

Description

There is issue in olympusmn.cpp , code execution cannot reach line (1) 1219 to 1223 and (2) 1209 to 1215

(1)Condition will be either of if(l1 != 1) or else if(l1 != 2) hence l1 != 3 code is dead.
(2) For all cases other than lo =1 is handled in first if (l0 != 1) condition hence only for l0 it goes inside else.

Associated revisions

Revision 3341
Added by Niels Kristian Bech Jensen over 2 years ago

Fix issue #970: Dead code in olympusmn.cpp.

History

#1 Updated by Mahesh Hegde almost 3 years ago

There is issue in olympusmn.cpp , code execution cannot reach line (1) 1219 to 1223 and (2) 1209 to 1215
*Condition will be either of if(l1 != 1) or else if(l1 != 2) hence l1 != 3 code is dead.
(2)* For all cases other than lo == 1 is handled in first if (l0 != 1) condition hence only for l0 = 1 it goes inside else.

#2 Updated by Robin Mills almost 3 years ago

  • Category set to metadata
  • Assignee set to Niels Kristian Bech Jensen
  • Priority changed from Low to Normal
  • Target version set to 0.25

Coverity and Mahesh are right. This is dead code. I don't know anything about the Olympus Maker Notes, however I suspect the != operators should be == instead.

Maybe Niels or Phil can comment on this.

    std::ostream& OlympusMakerNote::print0x1015(std::ostream& os, const Value& value, const ExifData*)
    {
        if (value.count() != 2 || value.typeId() != unsignedShort) {
            return os << value;
        }
        short l0 = (short)value.toLong(0);
        if (l0 != 1) {
            os << _("Auto");
        }
        else {
            short l1 = (short)value.toLong(1);
            if (l1 != 1) { // more likely to be ==
                switch (l0) {
                case 0: os << _("Auto"); break;
                default: os << _("Auto") << " (" << l0 << ")"; break;
                }
            }
            else if (l1 != 2) { // more likely to be ==
                switch (l0) {
                case 2: os << _("3000 Kelvin"); break;
                case 3: os << _("3700 Kelvin"); break;
                case 4: os << _("4000 Kelvin"); break;
                case 5: os << _("4500 Kelvin"); break;
                case 6: os << _("5500 Kelvin"); break;
                case 7: os << _("6500 Kelvin"); break;
                case 8: os << _("7500 Kelvin"); break;
                default: os << value; break;
                }
            }
            else if (l1 != 3) {  // more likely to be ==
                switch (l0) {
                case 0: os << _("One-touch"); break;
                default: os << value; break;
                }
            }
            else {
                return os << value;
            }
        }
        return os;
    } // OlympusMakerNote::print0x1015

#3 Updated by Robin Mills almost 3 years ago

  • Status changed from New to Assigned

#4 Updated by Phil Harvey almost 3 years ago

It looks to me like it should be == instead of != in 4 of the if statements. But there are a number of other differences from the ExifTool decoding. I think it should be l0 not l1 in all of the if statements, and l1 instead of l0 in the switch statements. Also, a single value should be allowed, and indicates "Auto" if the value is 1.

#5 Updated by Niels Kristian Bech Jensen over 2 years ago

  • Due date set to 31 Aug 2014
  • Status changed from Assigned to Resolved
  • % Done changed from 0 to 100

I agree with Phil that the code looked strange. I have fixed it accordingly. Please run another scan to check the code.

Best regards,
Niels Kristian Bech Jensen

#6 Updated by Andreas Huggel almost 2 years ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF

Redmine Appliance - Powered by TurnKey Linux