Re: Key-value file parser, howto get all groups and create loop from them

[David Nečas (Yeti) <yeti physics muni cz>]

On Tue, Aug 01, 2006 at 07:57:18PM +0200, rupert wrote:
> this is the part with the for loop ,
> i had the idea of using a +i in the gtk_table_attach_defaults comamnd for
> the placement, right now the for only has one iteration, with more it
> crashes..

The code is not selfcontained.  Unless the completion is
obvious, code samples should be sent in a form people can
compile -- and even better run -- them.  But anyway...

>    GtkWidget *crypto_keys;
>     gboolean *isfile=FALSE;
>     gchar *startgroup;
>     gchar **groups=NULL;
>     //GArray *groups=NULL;
>     gint i, *groupcounter=0, *grouplenght=NULL;
>     GString *device, *mapper, *mountpoint, *password, *keyfile;

Variables are declared as random types again (gchar* as
GString*).

>     crypto_keys = g_key_file_new();
>     g_key_file_load_from_file(crypto_keys,
> "/home/Programmierung/GTK_Projects/project_dmaster/src/crypto.ini",
> G_KEY_FILE_NONE, NULL) ;
>
>     groups = g_key_file_get_groups(crypto_keys, &grouplenght);

Bogus argument are passed to functions again (&grouplenght
is pointer to the pointer grouplenght, you need to declare
grouplenght as gint (and please call it grouplength)).

>     //g_print("\n %d", grouplenght);

And again (grouplenght).

>     for(i=0; i<1; i++)
>     {
>         //put tthe info from the keyfile in variables
>         //g_print("\n %s", groups[i]);
>         device = g_key_file_get_string(crypto_keys, groups[i], "device",
> NULL);
>         mapper = g_key_file_get_string(crypto_keys, groups[i],
> "mapperdevice", NULL);
>         mountpoint = g_key_file_get_string(crypto_keys, groups[i],
> "mountpoint", NULL);
>         password = g_key_file_get_string(crypto_keys, groups[i], "password",
> NULL);
>         keyfile = g_key_file_get_string(crypto_keys, groups[i], "keyfile",
> NULL);

Some memory is never freed (all the strings).

>        g_print("\n %s", device);
>         g_print("\n %s", mapper);
>         g_print("\n %s", mountpoint);
>         g_print("\n %s", password);
>         g_print("\n %s", keyfile);
>
>         //free the strings
>       //  g_strfreev(groups);

On the other hand some memory is attempted to free
repeatedly (although commented out).

>         crypto_mapper_button = gtk_button_new(); //1
>         gtk_box_pack_start(GTK_BOX(crypto_content_vbox), cryptohbox, FALSE,
> TRUE, 0);

Widgets (cryptohbox) are packed repeatedly to parents.

>         pic_mapper_device =  g_object_new(GTK_TYPE_IMAGE,"file",
> "pics/no.png", NULL);

Generic object construction is used where a mere mortal
would just do gtk_image_new_from_file().  It looks like
cargo cult programming.

>         gtk_button_set_image(crypto_mapper_button, pic_mapper_device);
>         gtk_button_set_relief(crypto_mapper_button, GTK_RELIEF_NONE);
>         gtk_container_add(GTK_CONTAINER(crypto_mapper_button),
> pic_mapper_device);

Images are added to buttons twice, in conflicting ways
(gtk_button_set_image() vs. gtk_container_add()).

>         crypto_mount_button = gtk_button_new(); //2
>         pic_drive_mounted =  g_object_new(GTK_TYPE_IMAGE,"file",
> "pics/mount.png", NULL);
>         gtk_button_set_image(crypto_mount_button, pic_drive_mounted );
>         gtk_button_set_relief(crypto_mount_button, GTK_RELIEF_NONE);
>         gtk_container_add(GTK_CONTAINER(crypto_mount_button),
> pic_drive_mounted);

Ditto.

>         crypto_device = gtk_label_new(device);    //3
>         crypto_volume_name = gtk_label_new(mapper);//4
>         crypto_mountpoint = gtk_label_new(mountpoint);//5
>
>         crypto_go_button = gtk_button_new_with_label("Go");
>
>         crypto_passHbox = gtk_hbox_new(FALSE, 0);//6
>         crypto_password = gtk_entry_new();
>         crypto_keyfiles = gtk_label_new(keyfile);
>
>         //decide if use a keyfile or enter the password
>         if(iskey)
>         gtk_box_pack_start(GTK_BOX(crypto_passHbox), crypto_keyfiles, FALSE,
> TRUE, 0);
>         else
>         gtk_box_pack_start(GTK_BOX(crypto_passHbox), crypto_password, FALSE,
> TRUE, 0);
>
>         gtk_box_pack_start(GTK_BOX(crypto_passHbox), crypto_go_button,
> FALSE, TRUE, 0);
>
>         //pack everything into the table
>         gtk_table_attach_defaults(crypto_table,
> GTK_WIDGET(crypto_mapper_button), 0,1,1,2);
>         gtk_table_attach_defaults(crypto_table,
> GTK_WIDGET(crypto_mount_button), 1,2,1,2);
>         gtk_table_attach_defaults(crypto_table, GTK_WIDGET(crypto_device),
> 2,3,1,2);

Weird typecasting is used (isn't crypto_device GtkWidget*
already?).

>         gtk_table_attach_defaults(crypto_table,
> GTK_WIDGET(crypto_volume_name), 3,4,1,2);
>         gtk_table_attach_defaults(crypto_table,
> GTK_WIDGET(crypto_mountpoint), 4,5,1,2);
>         gtk_table_attach_defaults(crypto_table, GTK_WIDGET(crypto_passHbox),
> 5,6,1,2);

As far I can infer the widget layout, it is strange.  Why
parts of the rows are packed to hboxes on their own instead
of cells of the table?

>         //abfrage des init und mountstatus
>     //    g_timeout_add(1000, crypto_out_device, pic_mapper_device);
>     //    g_timeout_add(1000, crypto_mount_check, pic_drive_mounted);
>         gtk_entry_set_visibility(crypto_password, FALSE);
>
>         //buttonpress events, replace the NULL with the device/mountpoint
> ...
>         g_signal_connect(G_OBJECT(crypto_mount_button),
> "button_press_event", G_CALLBACK(crypto_mount_drive), NULL);
>         g_signal_connect(G_OBJECT(crypto_mapper_button),
> "button_press_event", G_CALLBACK(crypto_remove_mapper), NULL);
>         g_signal_connect(G_OBJECT(crypto_go_button), "button_press_event",
> G_CALLBACK(crypto_init_mapper), NULL);

Wrong signals are used (in place of "clicked").

The G_OBJECT() typecast is again useless.

>         //eventbox
>
>         gtk_widget_set_events(crypto_mount_button, GDK_BUTTON_PRESS_MASK);

Events are removed from widgets, breaking them seriously.
Use gtk_widget_add_events() to adds events.  However, this
does not make sense here at all -- buttons can react to
mouse clicks well by design.

>         g_signal_connect(G_OBJECT(crypto_mount_button), "enter",
> G_CALLBACK(crypto_onmouse_in), GINT_TO_POINTER(crypto_buffer));
>         g_signal_connect(G_OBJECT(crypto_mount_button), "leave",
> G_CALLBACK(crypto_onmouse_out), GINT_TO_POINTER(crypto_buffer));

Nonexistent signals are used.

>         //function calls
>         //        dev_init_status = crypto_out_device(pic_mapper_device);
>         dev_init_status = crypto_out_device(pic_mapper_device);
>         crypto_mount_check(pic_drive_mounted);
>       //    g_print("%c", dev_init_status);

This is strange as crypto_out_device() is a GSourceFunc and
it is suppsed to return a gboolean, that is TRUE or FALSE,
printing it as %c is not likely to be useful.  Even if it
returned some other non-zero value for true, the FALSE case
is still strange.

>   }
>         //for end
>
> gtk_widget_show_all(crypto_table);

I'm afraid you started a project which exceeds your current
capabilities and/or level of knowledge.

Switch on all compiler warnings.  Or at least -Wall -W.  And
fix everything the compiler will complain about.  It will
tell you about many of the errors.  Learn to use gdb and
valrgind, they will tell you about more.

Learn Gtk+ by actually going through the tutorial, making
sure you understand the provided examples before you move
on.  Improving your C skills along the way would help too.
Switching the language can help marginally (mostly with
memory issues) -- many of the errors above are logical and
they can be repeated in any language.

If something doesn't work, check the documentation if you
use it as intended.

If something doesn't work, check the documentation if you
use it as intended (yes, it worths repeating).

Read other people's code.

Yeti


--
Anonyms eat their boogers.