Re: [gnome-network]PATCHES for bug 114826

[Rodrigo Moya <rodrigo gnome-db org>]

On Tue, 2003-12-09 at 03:53, Emil Soleyman-Zomalan wrote:
> On Sun, 2003-12-07 at 12:25, Rodrigo Moya wrote:
> 
> > this one looks perfect and has now been applied to CVS, except for the
> > .png file. Could you please resend that file, so that I add it to CVS?
> 
> You can find the PNG file at
> http://www.nishra.com/patches/gnome-remote-shell_window.png
> 
ok, this is now in CVS then.

> 
> > > http://www.nishra.com/patches/help-and-err.patch
> > >
> > this one has some small problems. For instance, you replaced all
> > #include's with an "#include gnome.h". While this is correct, it slows
> > down compilation, since a lot of unneeded header files are used during
> > the compilation. So please don't do that, keep the headers as they are.
> 
> I understand. My recent patch reverts this change.
> 
> > Also, please remove the big /*** for the functions you added. Those are
> > private functions, so there is no need to have them documented as you
> > did. Just add a quick comment (/* this does this */).
> 
> Done.
> 
> > Also, in some:
> > 
> > +			title = g_strdup_printf ("Failed to create a connection");
> > +			reason = g_strdup_printf ("The host cannot be found");
> > +			create_error (title, reason);
> > +
> > +			g_free (title);
> > +			g_free (reason);
> > 
> > there is really no reason at all to allocate memory for 'reason' :-) Just
> > pass the const string to the create_error function.
> 
> That's also done. I just thought that it was cleaner by creating a new
> variable and passing both title and reason to create_error(). But this
> also works. 
> 
> The new patch can be found at:
> http://www.nishra.com/patches/help-and-err-2.patch
>
it's still wrong :-(

>  			msgerror = (gchar *) strerror (errno);
>  			msgerror = g_locale_to_utf8 (msgerror, strlen (msgerror), NULL, NULL, NULL);
> -			gnome_error_dialog_parented (_(msgerror), GTK_WINDOW (dialog));
> +			
> +			title = g_strdup_printf ("There is a connection error");
> +			create_error (title, msgerror);
> +									
> +			g_free (title);
>
no need to allocate memory for 'title'. Also, you are missing marking
the strings for translation _("String"), and are leaking 'msgerror',
which should be freed, since the value returned by g_locate_to_utf8
is a new allocated string.

cheers