After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 393994 - use goption in font thumbnailer
use goption in font thumbnailer
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: [obsolete] fonts:///
git master
Other Linux
: Normal enhancement
: ---
Assigned To: James Henstridge
Control-Center Maintainers
Depends on: 333557
Blocks:
 
 
Reported: 2007-01-07 19:16 UTC by Christian Persch
Modified: 2007-02-12 20:13 UTC
See Also:
GNOME target: ---
GNOME version: Unversioned Enhancement


Attachments
patch (4.26 KB, patch)
2007-01-07 19:17 UTC, Christian Persch
none Details | Review
updated patch (4.29 KB, patch)
2007-01-07 19:33 UTC, Christian Persch
needs-work Details | Review
cleanup on error (5.77 KB, patch)
2007-02-10 15:14 UTC, Christian Persch
needs-work Details | Review
don't leak uri, and change goto label to "out" (5.77 KB, patch)
2007-02-12 19:44 UTC, Christian Persch
accepted-commit_now Details | Review

Description Christian Persch 2007-01-07 19:16:39 UTC
+++ This bug was initially created as a clone of Bug #333557 +++

The patch from bug 333557 added text string and font size support to the font thumbnailer. But the patch does the argument parsing ad-hoc, instead of using goption. I propose to use goption, and make the text and font-size regular options instead. Since the other patch was just committed, that shouldn't be a compatibility problem if this patch is applied before the next stable release.
Comment 1 Christian Persch 2007-01-07 19:17:14 UTC
Created attachment 79670 [details] [review]
patch
Comment 2 Christian Persch 2007-01-07 19:33:14 UTC
Created attachment 79672 [details] [review]
updated patch

Need to call setlocale.
Comment 3 Jens Granseuer 2007-02-10 13:59:34 UTC
You're possibly leaking "arguments" in all error paths.
Comment 4 Christian Persch 2007-02-10 14:19:28 UTC
Yes, as well as thumbstr_utf8. I'm not sure how critical that is... the easiest way would be a "goto error" and clean up there, instead of sprinkling it everywhere; what do you think?
Comment 5 Jens Granseuer 2007-02-10 14:29:03 UTC
Go for it. We've had a policy discussion for goto, and it's cleared for simple cleaning-up routine.
Comment 6 Christian Persch 2007-02-10 15:14:32 UTC
Created attachment 82283 [details] [review]
cleanup on error
Comment 7 Jens Granseuer 2007-02-10 15:57:12 UTC
Still leaking

    uri = gnome_vfs_make_uri_from_shell_arg (arguments[0]);

in the error case. And I'd prefer a more factual goto label...
Comment 8 Christian Persch 2007-02-12 19:44:45 UTC
Created attachment 82416 [details] [review]
don't leak uri, and change goto label to "out"
Comment 9 Christian Persch 2007-02-12 20:13:24 UTC
        * vfs-methods/fontilus/thumbnailer.c: (main): Use GOption argument
        parsing here, and fix some leaks in the error paths. Bug #393994.