Bug 20660 - Improved error reporting of 'ErrOpenFile' errors
Summary: Improved error reporting of 'ErrOpenFile' errors
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: glib frontend (show other bugs)
Version: unspecified
Hardware: Other All
: medium enhancement
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-03-14 06:22 UTC by Eric Toombs
Modified: 2009-03-22 14:50 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
improves error reporting in poppler-glib (3.85 KB, patch)
2009-03-14 06:22 UTC, Eric Toombs
Details | Splinter Review
this version leaves the multicase behaviour alone for the most part (see comment). (4.52 KB, patch)
2009-03-14 15:57 UTC, Eric Toombs
Details | Splinter Review
minor adjustments, added commentary (1.57 KB, patch)
2009-03-21 16:45 UTC, Eric Toombs
Details | Splinter Review

Description Eric Toombs 2009-03-14 06:22:02 UTC
Created attachment 23850 [details] [review]
improves error reporting in poppler-glib

I modified poppler so that if 'ErrOpenFile' occurs from running poppler_document_new_from_file(uri, password, error), the resulting GError will be a G_FILE_ERROR with the appropriate code and message for the circumstances. It is now possible, for instance, for a program using poppler-glib to distinguish 'file not found', 'no permission', 'is a directory', etc. errors.

I started from the git repository today and I've come up with the attached patch.
Comment 1 Eric Toombs 2009-03-14 06:35:18 UTC
This patch also gets rid of what I think many would consider undesirable behaviour allowing, to some extent, case-insensitive matching of file names. It used to try opening, for example, "fileName.Pdf", then failing that try opening "filename.pdf", then failing that try opening "FILENAME.PDF". This attempt at matching the case doesn't make sense in a case-sensitive file system. Also, it would try to open "filename.pdf" without even checking the reason why opening "fileName.Pdf" had failed in the first place, meaning that it could have initially been a problem with file permissions.
Comment 2 Albert Astals Cid 2009-03-14 11:47:57 UTC
Do not remove the multicase behaviour, if you want to propose that, send a separate patch in a separate bug.
Comment 3 Eric Toombs 2009-03-14 15:57:01 UTC
Created attachment 23865 [details] [review]
this version leaves the multicase behaviour alone for the most part (see comment).

This one is the same as the last one but with the asinine multicase behaviour reincorporated and made somewhat more sane. This time, it only tries a different name if the error returned is ENOENT (no such file or directory).
Comment 4 Albert Astals Cid 2009-03-21 10:48:37 UTC
Carlos what do you say about the glib side of the patch? I'm thinking of commiting it to trunk only btw
Comment 5 Eric Toombs 2009-03-21 16:45:44 UTC
Created attachment 24125 [details] [review]
minor adjustments, added commentary

This patch follows immediately after the patch that leaves the multicase behaviour alone. I made some minor adjustments and added some commentary.

By the way, Albert, what do you mean by 'trunk'. I thought that was a subversion thing.
Comment 6 Albert Astals Cid 2009-03-22 08:18:50 UTC
I'm a subversion man ;-) s/trunk/master, that is the development branch, not the stable (0.10.x) one

BTW the patch is a bit wrong, the error line should be filename->getCString and not fn->getCString() but no need to post a new patch i'll fix it when commiting once Carlos says the glib part is ok.
Comment 7 Carlos Garcia Campos 2009-03-22 08:22:20 UTC
(In reply to comment #4)
> Carlos what do you say about the glib side of the patch? I'm thinking of
> commiting it to trunk only btw
> 

looks good to me. 
Comment 8 Albert Astals Cid 2009-03-22 14:50:56 UTC
Commited


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.