Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

panic: division by zero #51

Open
mholt opened this issue Jan 11, 2017 · 7 comments
Open

panic: division by zero #51

mholt opened this issue Jan 11, 2017 · 7 comments

Comments

@mholt
Copy link

mholt commented Jan 11, 2017

I'm trying to track down how to reproduce this error, but:

panic: division by zero

goroutine 14 [running]:
panic(0x2d48a0, 0xc42120e7a0)
	/usr/local/go/src/runtime/panic.go:500 +0x1a1
math/big.(*Rat).SetFrac64(0xc420d99740, 0x0, 0x0, 0x13)
	/usr/local/go/src/math/big/rat.go:311 +0x147
math/big.NewRat(0x0, 0x0, 0x0)
	/usr/local/go/src/math/big/rat.go:25 +0x4f
github.com/rwcarlsen/goexif/tiff.(*Tag).Rat(0xc4207d4000, 0x0, 0x34b1b7, 0xb, 0xc420253dd8)
	/Users/matt/Dev/src/github.com/rwcarlsen/goexif/tiff/tag.go:329 +0x7a

So this line must mean that d is 0.

My calling code:

// altitude
rawAlt, err := x.Get(exif.GPSAltitude)
if err != nil {
	return nil, fmt.Errorf("getting altitude from EXIF: %v", err)
}
alt, err := rawAlt.Rat(0) // *boom*
if err != nil {
	return nil, fmt.Errorf("converting altitude value: %v", err)
}
altFlt, _ := alt.Float64()
@mholt mholt changed the title panic: division by 0 panic: division by zero Jan 11, 2017
@gmcnaughton
Copy link

Can you link to a sample image that repros the panic?

The docs for Rat say "It panics for zero deminators or if i is out of range.", so it sounds like the panic is expected, based on a zero denominator in the exif data (though maybe returning an error instead would make it easier to use!)

@mholt
Copy link
Author

mholt commented Jun 12, 2017

Hmm, I don't have the sample image anymore, I'd have to dig to find it. Which I could do, but... later.

I really wish it didn't panic. :) Libraries shouldn't panic. It should return an error value.

@gmcnaughton
Copy link

Totally agree!

FWIW, looking at the open pull requests, it looks like active development on this library has moved to https://github.com/xor-gate/goexif2.

It might be worth checking to see whether that fork has the same bug, and if so re-reporting it there.

@xor-gate
Copy link

I'm commenting on this old issue as i'm the author of goexif2 the fork of rwcarlsen/goexif. This issue is still not resolved but was resolved in goexif2. I have archived goexif2 as both projects are diverged. I would like to see all the patches I did integrated into rwcarlsen/goexif so it could be rock solid.

@rwcarlsen what do you think about merging the patches I did on goexif2 to make it more mature back to goexif?

@mholt
Copy link
Author

mholt commented Mar 18, 2022

@rwcarlsen Any thoughts on that? ^

@rwcarlsen
Copy link
Owner

That sounds fine to me. @mholt Would you be willing to take lead? I can add you as a collaborator here. I'm busy trying to build a house in my spare time right now.

@mholt
Copy link
Author

mholt commented May 25, 2022

@rwcarlsen Thanks for the invitation. I am a bit busy lately myself; but if I can circle back to this I will let you know!

This is a tough time to build a house, best of luck! That's exciting though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants