[virt-tools-list] [PATCH 00/10] Install Scripts fixes and improvements

[Christophe Fergeau]

Hey,

On Wed, Jun 13, 2012 at 04:28:44AM +0300, Zeeshan Ali (Khattak) wrote:
>    * We don't want any os-specifics in apps and this function does
> some very specific hard-coding.

This is indeed an interesting ideal goal, not having looked at this code at
all means I have no idea how doable this is. However, as far as I'm
concerned, even if we still need a bit of OS specific code in apps, having
the autoinstall code in a generic library that others can reuse is already
a good move.

>    * It sets a global variable, while it can just return that value to
> caller and that value could be passed around.
>  * !(strcmp(..)) -> strcmp(..) == 0
>  * This code:
> 
>    gsize len = sizeof(distro) + sizeof(".ks");
>    gchar *output = g_malloc(len);
>    g_snprintf(output, len, "%s.ks", distro);
> 
>    can be replaced by:
> 
>    gchar *output = g_strjoin(".", distro, "ks", NULL);

gchar *output = g_strdup_printf("%s.ks", distro) is closer to the initial
code and more readable imo.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20120613/c64d86ab/attachment.sig>