Re: Code sugestions ... (patchletes?)

[Jeff Garzik <jgarzik pobox com>]

Don't mistake me for a GTK+ or GLib expert, but here's some comments...

* looks good.  :)  You are definitely going in the right direction, and
  more developers should definitely page through glib.h (or
  http://glade.pn.org/glibdocs/) and use the functions there.
  g_get_home_dir() is only one of the many convenience functions under
  the "misc" category that are damn useful.

* Ideally, any call to the printf family (printf, sprintf, ..) should 
  be prefixed with g_.

* Any sprintf calls should be replaced with g_strdup_printf or g_snprintf.
  In the below example, you can replace g_malloc()+sprintf() with a
  single call to g_strdup_printf().

	Jeff



Marin Purgar - PMC wrote:
> I have two simple code suggestions for gtk+ code I recently checked out
> from CVS.

> Firs I see that there is getenv("HOME") call in gtkrc.c. glib provides
> g_get_home_dir call. Wouldn't it be cleaner to use it ?
> 
> --- gtkrc.c.orig Thu Nov 12 00:37:56 1998
> +++ gtkrc.c Thu Nov 12 00:51:52 1998
> @@ -292,7 +292,7 @@
>      }
>    module_path[n++] = g_strdup(path);
>    g_free(path);
> -  var = getenv("HOME");
> +  var = g_get_home_dir();
>    if (var)
>      {
>        path = g_malloc(strlen(var) + strlen(".gtk/lib/themes/engines")
> +1);
> 
> And when referencing user theme libs one slash is missing in the concat:
> 
> --- gtkrc.c.orig Thu Nov 12 00:37:56 1998
> +++ gtkrc.c Thu Nov 12 00:52:45 1998
> @@ -295,8 +295,8 @@
>    var = getenv("HOME");
>    if (var)
>      {
> -      path = g_malloc(strlen(var) + strlen(".gtk/lib/themes/engines")
> +1);
> -      sprintf(path, "%s%s", var, ".gtk/lib/themes/engines");
> +      path = g_malloc(strlen(var) + strlen("/.gtk/lib/themes/engines")
> +1);
> +      sprintf(path, "%s%s", var, "/.gtk/lib/themes/engines");
>      }
>    module_path[n++] = g_strdup(path);
>    module_path[n] = NULL;
> 
> Also IMHO g_string_sprintf should be used instead of sprintf (or not?).
> I am fairly new to glib so this may be only babble of beginner ;) ...