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 559163 - [font] cleanup module
[font] cleanup module
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: plugins
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
: 310089 530975 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2008-11-03 19:04 UTC by Behdad Esfahbod
Modified: 2008-11-03 21:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
the patch (11.52 KB, patch)
2008-11-03 19:13 UTC, Behdad Esfahbod
none Details | Review

Description Behdad Esfahbod 2008-11-03 19:04:21 UTC
The old code had several flaws:

  - It tried to create directories in user's home even if we didn't have
    any use for them.

  - It called mkfontdir and XSync even if there was no fonts installed.

The new code does the following:

  - Only call mkfontdir and XSync if there's actually any fonts in the
    relevant dirs.

  - Remove the ~/.gnome2/share/fonts and/or ~/.gnome2/share/cursor-fonts
    if they are empty and no cursor font is set.

This should fix bugs 310089 and part of 530975.
Comment 1 Behdad Esfahbod 2008-11-03 19:13:12 UTC
Created attachment 121902 [details] [review]
the patch
Comment 2 Behdad Esfahbod 2008-11-03 19:15:07 UTC
*** Bug 530975 has been marked as a duplicate of this bug. ***
Comment 3 Behdad Esfahbod 2008-11-03 19:15:47 UTC
*** Bug 310089 has been marked as a duplicate of this bug. ***
Comment 4 Jens Granseuer 2008-11-03 20:57:28 UTC
+        if (! g_file_test (font_dir, G_FILE_TEST_EXISTS)) {
+		if (create) {
+			if (g_mkdir_with_parents (font_dir, 0755) != 0) {
+				g_warning ("Cannot create needed directory \"%s\".\n", font_dir);
+				g_free (font_dir);
+				return NULL;
+			}
+		} else {
+			g_free (font_dir);
+			return NULL;
+		}

Why create font_dir and stat it if you only need it when create == TRUE?

+				g_warning ("Cannot create needed directory \"%s\".\n", font_dir);

No trailing \n, please.

+	/* remove the fonts.dir and fonts.scale files that mkfontdir generate. */

s/generate/generates/

+                (void) symlink (cursor_font, newpath);

What's that supposed to mean?

Oh, and broken indentation, of course. :-P
Comment 5 Behdad Esfahbod 2008-11-03 21:03:48 UTC
(In reply to comment #4)
> +        if (! g_file_test (font_dir, G_FILE_TEST_EXISTS)) {
> +               if (create) {
> +                       if (g_mkdir_with_parents (font_dir, 0755) != 0) {
> +                               g_warning ("Cannot create needed directory
> \"%s\".\n", font_dir);
> +                               g_free (font_dir);
> +                               return NULL;
> +                       }
> +               } else {
> +                       g_free (font_dir);
> +                       return NULL;
> +               }
> 
> Why create font_dir and stat it if you only need it when create == TRUE?

Humm, where do I create and stat before checking create == TRUE?  Sure, we stat first.  Oh, I see.  If create == TRUE I don't need the g_file_test.  Fixing.

> +                               g_warning ("Cannot create needed directory
> \"%s\".\n", font_dir);
> 
> No trailing \n, please.

Eh, right.  Copied the old message.

> +       /* remove the fonts.dir and fonts.scale files that mkfontdir generate.
> */
> 
> s/generate/generates/

Ack.

> +                (void) symlink (cursor_font, newpath);
> 
> What's that supposed to mean?

The (void) is supposed to silence the gcc warning about unused results for symlink, but it doesn't actually do that.  I'll remove it.

> Oh, and broken indentation, of course. :-P

Haha.
Comment 6 Behdad Esfahbod 2008-11-03 21:25:48 UTC
All fixed.

2008-11-03  Behdad Esfahbod  <behdad@gnome.org>

        * plugins/font/gsd-font-manager.c (setup_dir), (empty_check_dir),
        (setup_font_dir), (setup_cursor_dir), (load_font_paths),
        (gsd_font_manager_start): Cleanup font module (bug #559163)

        The old code had several flaws:

        - It tried to create directories in user's home even if we didn't have
          any use for them.

        - It called mkfontdir and XSync even if there was no fonts installed.

        The new code does the following:

        - Only call mkfontdir and XSync if there's actually any fonts in the
          relevant dirs.

        - Remove the ~/.gnome2/share/fonts and/or ~/.gnome2/share/cursor-fonts
          if they are empty and no cursor font is set.