[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: gtk frontend for cdebconf, ready for commit



tor 2002-09-05 klockan 02.31 skrev Michael Cardenas:

Now I've tested it, and I get a segfault after the password dialog.
Let's have a look at some of the code. I hope my comments are useful ;)

> +void passwd_callback( GtkWidget *widget,
> +                     GtkWidget *entry )
> +{
> +  const char *entrytext;
> +  entrytext = gtk_entry_get_text (GTK_ENTRY (entry));
> +  
> +  gPassword = malloc(strlen(entrytext));
> +  strcpy(gPassword, entrytext);

Why not just strdup?

> +  printf("Password captured.\n");
> +}

> +gint delete_event( GtkWidget *widget,
> +                   GdkEvent  *event,
> +		   gpointer   data )
> +{
> +  /* If you return FALSE in the "delete_event" signal handler,
> +   * GTK will emit the "destroy" signal. Returning TRUE means
> +   * you don't want the window to be destroyed.
> +   * This is useful for popping up 'are you sure you want to quit?'
> +   * type dialogs. */
> +
> +  g_print ("delete event occurred\n");
> +
> +  /* Change TRUE to FALSE and the main window will be destroyed with
> +   * a "delete_event". */
> +
> +  return TRUE;
> +}

As I said, clicking Finish on a dialog box doesn't close the dialog box.
I'm not sure this is good or bad. I think it gets cluttered fast.

> +/*
> + * Function: gtkhandler_boolean
> + * Input: struct frontend *obj - frontend object
> + *        struct question *q - question to ask
> + * Output: int - DC_OK, DC_NOTOK
> + * Description: handler for the boolean question type
> + * Assumptions: none
> + */
> +static int gtkhandler_boolean(struct frontend *obj, struct question *q)
> +{
> +	int ans = -1;
> +	int def = -1;
> +	const char *defval;
> +	GtkWidget *window;
> +	GtkWidget *boolButton;
> +	GtkWidget *hboxtop, *hboxbottom, *bigbox;
> +
> +	defval = question_defaultval(q);
> +	if (defval)
> +	{
> +		if (strcmp(defval, "true") == 0)
> +			def = 1;
> +		else 
> +			def = 0;
> +	}
> +
> +	//create the window for the question
> +	window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
> +
> +	bigbox = gtk_vbox_new(FALSE, 0);
> +	hboxtop = gtk_hbox_new(0, 0);
> +	hboxbottom = gtk_hbox_new(0, 0);
> +
> +	//add a checkbox for the boolean, we wouldn't really want a whole dialog for one boolean,
> +	// but this is a start, in the future, I'll have to step through the questions and make
> +	// intelligent groupings
> +
> +	boolButton = gtk_check_button_new_with_label ( (gchar *)question_description(q) );
> +	g_signal_connect (G_OBJECT (boolButton), "clicked",
> +			  G_CALLBACK (check_callback), NULL);

Instead of global variables, why not pass in a pointer to ans instead of
NULL? That's the 'data' argument in the callback, right?

> +	gtk_box_pack_start(GTK_BOX(hboxtop), boolButton, FALSE, 5, 5);
> +	gtk_widget_show (boolButton);		
> +
> +	add_common_buttons(window, hboxbottom, bigbox, q);
> +
> +	gtk_box_pack_start(GTK_BOX(bigbox), hboxtop, FALSE, 5, 5);
> +	gtk_box_pack_start(GTK_BOX(bigbox), hboxbottom, FALSE, 5, 5);
> +
> +	gtk_container_add(GTK_CONTAINER(window), bigbox);
> +
> +	//show the dialog containing the question
> +	gtk_widget_show (hboxbottom);	
> +	gtk_widget_show (hboxtop);		
> +	gtk_widget_show (bigbox);		
> +	gtk_widget_show (window);
> +
> +	gtk_main ();
> +
> +	if(gBool)
> +	  {
> +	    ans = 1;
> +	  }
> +	else
> +	  {
> +	    ans = 0;
> +	  }

Meaning you wouldn't have to do this...

> +	question_setvalue(q, (ans ? "true" : "false"));
> +	return DC_OK;
> +}

> +static int gtkhandler_multiselect(struct frontend *obj, struct question *q)
> +{
> +	char *choices[100] = {0};
> +	char *defaults[100] = {0};
> +	char selected[100] = {0};
> +	char answer[1024];
> +	int i, j, count, dcount;
> +	GtkWidget *window;
> +	GtkWidget *hboxtop, *hboxbottom, *bigbox;
> +	GtkWidget **choiceButtons;
> +
> +	count = strchoicesplit(question_choices(q), choices, DIM(choices));
> +	dcount = strchoicesplit(question_defaultval(q), defaults, DIM(defaults));
> +
> +	choiceButtons = (GtkWidget **)malloc( (count * (sizeof(GtkWidget *)) + 1) );

Hmm, shouldn't that be  count * (sizeof(GtkWidget*) + 1)? You want n+1
pointers, don't you?

> +	for(i = 0; i < count; i++)
> +	  choiceButtons[i] = (GtkWidget *)malloc(sizeof(GtkWidget *));

Eh? Why do you want to do this? You want to use gtk_*_new functions to
do this.

> +	choiceButtons[count + 1] = 0;

And this should be [count], since it's zero-based, right?

> +	if (dcount > 0)
> +		for (i = 0; i < count; i++)
> +			for (j = 0; j < dcount; j++)
> +				if (strcmp(choices[i], defaults[j]) == 0)
> +					{
> +					  selected[i] = 1;
> +					  gMultiChoices[i] = 1;
> +					}
> +
> +	//create the window for the question
> +	window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
> +	bigbox = gtk_vbox_new(FALSE, 0);
> +	hboxbottom = gtk_hbox_new(0, 0);
> +	hboxtop = gtk_vbox_new(0, 0);
> +
> +	//add the next and back buttons
> +
> +	add_common_buttons(window, hboxbottom, bigbox, q);
> +
> +	for (i = 0; i < count; i++)
> +	  {
> +	    //create a button for each choice here and init the global place to store them
> +
> +	    choiceButtons[i] = gtk_check_button_new_with_label ( (gchar *)choices[i] );

And you "overwrite" the pointers here, so your malloc was pretty
unnecessary in the first place :)

> +	    g_signal_connect (G_OBJECT (choiceButtons[i]), "clicked",
> +			      G_CALLBACK (multicheck_callback), choiceButtons);

This is where you'd want a higher-level language so you could pass in a
list of pairs instead <wink> Seriously, a list of structs with both
widget and result value might be nice? I don't know how allergic others
are to global variables, but I'm pretty damn allergic ;) Another idea
would be to actually create our own full-blown gtk2 type and create an
object for it to make it even more object-oriented. Just an idea :)

> +	    gtk_box_pack_start(GTK_BOX(hboxtop), choiceButtons[i], FALSE, 5, 0);
> +	    gtk_widget_show (choiceButtons[i]);
> +			
> +	  }
> +
> +	gtk_box_pack_start(GTK_BOX(bigbox), hboxtop, FALSE, 5, 5);
> +	gtk_box_pack_start(GTK_BOX(bigbox), hboxbottom, FALSE, 5, 5);
> +
> +	//add the main box to the layout
> +	gtk_container_add(GTK_CONTAINER(window), bigbox);
> +
> +	//show the dialog containing the question
> +	gtk_widget_show (hboxbottom);	
> +	gtk_widget_show (hboxtop);		
> +	gtk_widget_show (bigbox);		
> +	gtk_widget_show (window);
> +
> +	gtk_main();
> +
> +	//once the dialog returns, handle the result
> +
> +	*answer = 0;
> +	j = 0;
> +
> +	for (i = 0; i < count; i++)
> +	  {
> +	    if(gMultiChoices[i])
> +	      {
> +		if(j)
> +		  {
> +		    strcat(answer, ", ");
> +		  }
> +
> +		strcat(answer, choices[i]);
> +		j++;
> +	      }
> +	  }
> +
> +	for (i = 0; i < count; i++)
> +		free(choiceButtons[i]);

This is where it segfaults.

> +	printf("%s", answer);
> +
> +	question_setvalue(q, answer);
> +	
> +	return DC_OK;
> +}

> +static int gtkhandler_password(struct frontend *obj, struct question *q)
> +{
> +	char passwd[256] = {0};
> +	GtkWidget *window, *hboxbottom, *hboxtop, *bigbox;
> +	GtkWidget *entry;
> +
> +	//create the window for the question
> +	window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
> +	bigbox = gtk_vbox_new(FALSE, 0);
> +	hboxbottom = gtk_hbox_new(0, 0);
> +	hboxtop = gtk_hbox_new(0, 0);
> +
> +	add_common_buttons(window, hboxbottom, bigbox, q);
> +
> +	//add actual password field
> +	entry = gtk_entry_new ();
> +	gtk_entry_set_max_length (GTK_ENTRY (entry), 50);
> +	g_signal_connect (G_OBJECT (entry), "activate",
> +			  G_CALLBACK (passwd_callback),
> +			  entry);

Err. Why pass in entry here? Won't entry be 'widget' in the callback?

> +	gtk_box_pack_start (GTK_BOX (hboxtop), entry, TRUE, TRUE, 0);
> +	gtk_widget_show (entry);
> +
> +	gtk_entry_set_visibility(GTK_ENTRY(entry), FALSE);
> +
> +	gtk_box_pack_start(GTK_BOX(bigbox), hboxtop, FALSE, 5, 5);
> +	gtk_box_pack_start(GTK_BOX(bigbox), hboxbottom, FALSE, 5, 5);
> +
> +	//add main box to layout
> +	gtk_container_add(GTK_CONTAINER(window), bigbox);
> +
> +	//show the dialog containing the question
> +	gtk_widget_show (hboxbottom);	
> +	gtk_widget_show (hboxtop);		
> +	gtk_widget_show (bigbox);		
> +	gtk_widget_show (window);
> +
> +	gtk_main();
> +
> +	strcpy(passwd, gPassword);

This segfaults if gPassword is NULL.

> +	question_setvalue(q, passwd);
> +	return DC_OK;
> +}

> +static int gtkhandler_select(struct frontend *obj, struct question *q)

Many of the comments for multiselect apply here too.

> +static int gtkhandler_string(struct frontend *obj, struct question *q)

And here, for passwd.

> +static int gtkhandler_text(struct frontend *obj, struct question *q)
> +{
> +	char buf[1024];
> +	GtkWidget *window;
> +	GtkWidget *hboxtop, *hboxbottom, *bigbox;
> +	GtkWidget *view;
> +	GtkTextBuffer *buffer;
> +        char *text;
> +        GtkTextIter start, end;
> +
> +	//create the window for the question
> +	window = gtk_window_new (GTK_WINDOW_TOPLEVEL);
> +	bigbox = gtk_vbox_new(FALSE, 0);
> +	hboxbottom = gtk_hbox_new(0, 0);
> +	hboxtop = gtk_vbox_new(0, 0);
> +
> +	add_common_buttons(window, hboxbottom, bigbox, q);
> +
> +	//add the text entry field
> +	view = gtk_text_view_new ();
> +	gtk_box_pack_start(GTK_BOX(hboxtop), view, FALSE, 5, 5);
> +	gtk_widget_show (view);	
> +
> +	buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (view));
> +	gtk_text_buffer_get_start_iter (buffer, &start);
> +	gtk_text_buffer_get_end_iter (buffer, &end);
> +
> +	//adjust the size
> +      
> +	gtk_widget_set_size_request(view, 200, 100);
> +
> +	//add the boxes to the main layout object
> +	gtk_box_pack_start(GTK_BOX(bigbox), hboxtop, FALSE, 5, 5);
> +	gtk_box_pack_start(GTK_BOX(bigbox), hboxbottom, FALSE, 5, 5);
> +
> +	//add main box to layout
> +	gtk_container_add(GTK_CONTAINER(window), bigbox);
> +
> +	//show the dialog containing the question
> +	gtk_widget_show (hboxbottom);	
> +	gtk_widget_show (hboxtop);		
> +	gtk_widget_show (bigbox);		
> +	gtk_widget_show (window);
> +
> +	gtk_main();
> +
> +	text = gtk_text_buffer_get_text(buffer, &start, &end, FALSE);
> +
> +	printf("text captured: %s.\n", text);
> +
> +	strcpy(buf, text);

This segfaults too. But this time I did enter text!

> +	question_setvalue(q, buf);  
> +
> +	return DC_OK;
> +}

Attachment: signature.asc
Description: Detta =?ISO-8859-1?Q?=E4r?= en digitalt signerad meddelandedel


Reply to: