Re: [PATCH] X-Operating System in outgoing mail's header

[christophe barbe <christophe barbe ml online fr>]

Le 2001.08.28 10:54:09 +0200, Ali Akcaagac a écrit :
> hello,
> 
> i am just playing around with your patch and gonna apply
> that one within the next minutes but here a comment as far.
> 

Cool.

> 
> 
> > +    gchar * tmp, *format;
> > +
> > +    format=g_strdup(gtk_entry_get_text(GTK_ENTRY(pui->SystemIdFormat)));
> > +    tmp=get_SystemId(format);
> > +    gtk_label_set_text(GTK_LABEL (pui->SystemId_preview), tmp);
> > +    g_free(tmp);
> > +    g_free(format);
> 
> gchar *tmp = NULL;
> gchar *format = NULL;
> 
> ...
> ...
> ...
> 
> if (tmp == NULL)
>     g_free (tmp);
> if (format == NULL)
>     g_free (format);

here you mean tmp !=NULL    (-;
 
> would be the better solution in case if one of them failed.
> e.g. if g_strdup failed or if get_SystemId failed. if one of
> the pointers arent getting initialized then you free an
> already empty pointer. this causes a warning printed out on
> console.

I will change this.

> > +    result=(char *)malloc((len+1)*sizeof(char));
> > +    if (result==NULL) {
> > +        fprintf(stderr, "unable to allocate memory\n");
> > +        return NULL;
> > +    }
> 
> g_malloc in case of compatibility.

If I need to gnomify my mind too.

> > +    snprintf(tooltip_str, TOOLTIPSZ,
> > +        "You can use the following sustitutions" \
> > +        " to describe your system :\n" \
> > +        "%%m machine type (%s)\n" \
> > +        "%%h machine's hostname (%s)\n"  \
> > +        "%%s operating system name (%s)\n" \
> > +        "%%r operating system release (%s)\n" \
> > +        "%%v operating system version (%s)\n" \
> > +        ,balsa_app.uname.machine,
> > +        balsa_app.uname.nodename,
> > +        balsa_app.uname.sysname,
> > +        balsa_app.uname.release,
> > +        balsa_app.uname.version
> > +        );
> 
> this one too. placing the string into an array is quite evil,
> since you don't know how long the array is and if everything
> fits inside. i recommend you use g_strdup_printf as far as i
> remember theres a more equivalent version of what i recommend
> but i am not sure right now this gives you a pointer to a string
> back that is more accurate.

This sound good to me. I will try it.

> at least there are quite a lot of standard 'C' functions used
> this works but it would be better to GNOMIFY them with the
> replacement gnome functions. this also offers special compatibility
> on other plattforms etc.

Fully agree but I need to find out what is available in g stuff.

Thank you for your review.
Christophe

> -- 
> Name....: Ali Akcaagac
> Status..: Student Of Computer & Economic Science
> E-Mail..: mailto:ali.akcaagac@stud.fh-wilhelmshaven.de
> WWW.....: http://www.fh-wilhelmshaven.de/~akcaagaa
> 
> _______________________________________________
> balsa-list mailing list
> balsa-list@gnome.org
> http://mail.gnome.org/mailman/listinfo/balsa-list
> 
-- 
Christophe Barbé <christophe.barbe@online.fr>
GnuPG FingerPrint: E0F6 FADF 2A5C F072 6AF8  F67A 8F45 2F1E D72C B41E