Bug 159522

> Image: http://xtalk.msk.su/photo/Valday07/IMGP9854-01.jpeg

confirm it crashes, i will take a look at it thanks.

Angelo

Andreas,

This is relevant on Exiv2 library used by Gwenview. I CC you in this file to have your viewpoint. Thanks in advance...

Gilles Caulier

mercoledì 19 marzo 2008 alle 10:33, Gilles Caulier ha scritto:
[bugs.kde.org quoted mail]
The failure is for:
Program received signal SIGFPE, Arithmetic exception.
Andreas, do you rise such an exception? if, the failing code is :

int JPEGContent::dotsPerMeterX() const {
        return dotsPerMeter("XResolution");
}
    
    
int JPEGContent::dotsPerMeterY() const {
        return dotsPerMeter("YResolution");
}
    
    
int JPEGContent::dotsPerMeter(const QString& keyName) const {
        Exiv2::ExifKey keyResUnit("Exif.Image.ResolutionUnit");
        Exiv2::ExifData::iterator it = d->mExifData.findKey(keyResUnit);
        if (it == d->mExifData.end()) {
                return 0;
        }
        int res = it->toLong();
        QString keyVal = "Exif.Image." + keyName;
        Exiv2::ExifKey keyResolution(keyVal.ascii());
        it = d->mExifData.findKey(keyResolution);
        if (it == d->mExifData.end()) {
                return 0;
        }
        // The unit for measuring XResolution and YResolution. The same unit is used for both XResolution and YResolution.
        //     If the image resolution in unknown, 2 (inches) is designated.
        //         Default = 2
        //         2 = inches
        //         3 = centimeters
        //         Other = reserved
        const float INCHESPERMETER = (100. / 2.54); 
        switch (res) {
        case 3:  // dots per cm 
                return (it->toLong() * 100); 
        default:  // dots per inch 
                return (it->toLong() * INCHESPERMETER); 
        }
    
        return 0;
}

Maybe i forgot to catch exceptions..., the failing line is  return (it->toLong() * INCHESPERMETER); 

Angelo

martedì 18 marzo 2008 alle 15:01, paul s.romanchenko ha scritto:
> x-Resolution        |300/0
> y-Resolution        |300/0



exiv2 -Ptn /home/anaselli/tmp/gv_crash/IMGP9854-01.jpeg
Make                         PENTAX Corporation
Model                        PENTAX *ist DS
XResolution                  (300/0)
YResolution                  (300/0)
ResolutionUnit               inch
ExifTag                      90
ExposureTime                 1/750 s
FNumber                      F9.5
ExposureProgram              Auto
ISOSpeedRatings              400
DateTimeOriginal             2007:04:15 16:14:25
DateTimeDigitized            2007:04:15 16:14:25
ExposureBiasValue            0
MeteringMode                 Multi-segment
Flash                        No, compulsory
FocalLength                  150.0 mm
SensingMethod                One-chip color area
CustomRendered               Normal process
ExposureMode                 Auto
WhiteBalance                 Auto
FocalLengthIn35mmFilm        225.0 mm
SceneCaptureType             Standard
Contrast                     Normal
Saturation                   Normal
Sharpness                    Normal
SubjectDistanceRange         Distant view


hmmmm: 
XResolution                  (300/0)
YResolution                  (300/0)
300/0  is a nan....
From a pictures of mine:
XResolution                  180
YResolution                  180

It should be an exiv2 upstream bug i believe.

Angelo

Well, if I understand this correctly, the program calls toLong() on a Rational with denominator 0. The conversion of a Rational to a long is a simple division (and truncation). If the denominator is 0, the result of the division is an arithmetic exception, this is the usual behaviour in such a situation. Do you see a better way for Exiv2 to handle this?
I think this needs to be fixed in the application, but rather than catching the exception, I would check the value before attempting a conversion.

-ahu.

> Well, if I understand this correctly, the program calls toLong() on a Rational with denominator 0. 
> The conversion of a Rational to a long is a simple division (and truncation). If the denominator is 0,
> the result of the division is an arithmetic exception, this is the usual behaviour in such a situation. 
> Do you see a better way for Exiv2 to handle this?   

Well no. But my mail was written before understanding what the problem was ;)
> I think this needs to be fixed in the application, but rather than catching the exception, I would check the value before attempting a conversion.

I'm not sure of that, how can i know if that is a string that cannot be converted
before doing that? I mean i would like to trust toLong method instead...
something like QString::toLong does (or say to :) ) for instance.

Created attachment 23966 [details]
patch to avoid crashing on not well formed information

Aurelien, i made this patch to fix the problem since we cannot trust to toLong
call. A better way could be to interpret the string and get the number instead
of using QString::toLong that should return 0. Please tell me if you have a
better idea or i can commit it, thanks.
Angelo

> QString::toLong that should return 0

Are you saying QString::toLong for "300/0" returns 0? But that's mathematically not correct! And it would make the application believe everything is fine when it isn't. Does it return the correct value for "300/2" then?

Anyway, the tags that you're dealing with here are X/YResolution and should be Rational values, you could try something like this (Value::toRational() doesn't throw for any type of Value):

    Exiv2::Rational r = it->toRational();
    if (r.second == 0) {
        // do whatever is appropriate: signal an error to the caller,
        // silently ignore the problem, or even set r.second = 1
        // (which would probably fix the invalid example at hand...)
    }
    return (it->toLong() * INCHESPERMETER);

-ahu.

mercoledì 19 marzo 2008 alle 15:47, Andreas Huggel ha scritto:
[bugs.kde.org quoted mail]
Well as you said after it could return the right value or -1 a not valid one for instance.
A good test is better than a crash (i've tried to catch the exception but it seemed i couldn't :( )
 
> Are you saying QString::toLong for "300/0" returns 0? 
> But that's mathematically not correct! 

Well neither is 300/0 :p

> Anyway, the tags that you're dealing with here are X/YResolution and 
> should be Rational values,

hmm good to know it, i've never thought about that, i've always thought it was an int :( 
so do you mean it is something like float? I don't understand that though x/y resolution
what is the difference between 300/0 and 300/1 then? and shouldn't 300/2 written as 150?
> you could try something like this (Value::toRational() doesn't throw for any type of Value):  
> 
>     Exiv2::Rational r = it->toRational();
>     if (r.second == 0) {
>         // do whatever is appropriate: signal an error to the caller,
>         // silently ignore the problem, or even set r.second = 1
>         // (which would probably fix the invalid example at hand...)
>     }
>     return (it->toLong() * INCHESPERMETER);

Thanks for the hint i can make a better patch now.

Angelo

Created attachment 23968 [details]
a better patch to avoid crashing on not well formed information

used a rational value instead.

Patch looks good (except I prefer to use 'double' instead of 'float'). Please commit.

SVN commit 791583 by anaselli:

Fixed wrong exiv2 interface usage
BUG: 159522

 M  +8 -3      jpegcontent.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=791583