Discussion:
incorrect button label text in Fl_Pack
marty moore
2013-04-03 01:35:23 UTC
Permalink
Hi all,

I was playing around with dynamic buttons in a Fl_Pack under fltk-1.3.
My sample program is below.

what's supposed to happen:
1. create 5 buttons with label showing the number as they are creates.
2. when clicked, the user has the option to either add a new button (with the next integer as text), or delete the clicked button.

what happens:
1. The labels don't show the proper text - should be X,1,2,3,4 but shows up X,4,3,3,4.
2. When the delete menu item is clicked, the clicked button is deleted properly.
3. When the add menu item is clicked, a new button appears, but has garbage text.

I'm running fltk-1.3 on debian 2.5, with gcc-4.4.5-1, and emacs 23

thanks for any insight on this.
Marty

my test program:

#include <iostream>
#include <string>
#include <cstdio>
#include <cstdlib>

#include <FL/Fl.H>
#include <FL/Fl_Box.H>
#include <FL/Fl_Menu_Button.H>
#include <FL/Fl_Menu_Item.H>
#include <FL/Fl_Pack.H>
#include <FL/Fl_Window.H>

using std::cout;
using std::endl;
using std::string;

Fl_Pack* G1;
Fl_Window* WIN;

int idx = 0;

static Fl_Color LBG = FL_DARK_BLUE;
static Fl_Color LFG = FL_WHITE;
static Fl_Color LSC = FL_BLUE;;
static Fl_Color LTC = FL_WHITE;
static Fl_Font LFONT = FL_BOLD;
static Fl_Fontsize FSIZE = 30;
static int LSPC = 5;


string int2str (int num)
{
int size = sizeof(int);
char cstr[size];
sprintf(cstr, "%i", num);
string str = cstr;
return str;
}

class Sbut : public Fl_Menu_Button
{
public:
Sbut (int i);
~Sbut ();

int i() { return _i; }
void set( string str ) { label(str.c_str()); }
int _i;
};


//hhhhhhhhhhhhhhhhhhhhhhhhhhhh
void test_2 (Fl_Widget* w, void* v)
{
char* vv = (char*)v;

cout << "test_2: " << w->label()
<< ": " << vv << endl;
// Sbut* sb = (Sbut*)w;
if ( vv == "add" )
{
cout << "test_2: add" << endl;
Sbut* nb = new Sbut(idx);
G1->add(nb);
// G1->child(idx)->show();
idx++;
}
if ( vv == "del" )
{
cout << "test_2: delete" << endl;
G1->child(G1->find(w))->hide();
}
WIN->redraw();
}


//ddddddddddddddddddddddddddddddddddddd
Fl_Menu_Item xMenu[] = {
{"add button", 0, test_2, (void*)"add"},
{"delete button", 0, test_2, (void*)"del"},
{0}
};


//cccccccccccccccccccccccccc
Sbut::Sbut (int id )
: Fl_Menu_Button( 0, 0, 100, 50)
{
cout << "Sbut(s) 0: " << id << endl;
_i = idx;
string s = int2str(id);
cout << "str = " << s << endl;
label(s.c_str());
cout << "label = " << label() << endl;
color( LBG, LSC);
labelcolor(LFG);
labelfont(LFONT);
labelsize(FSIZE);
menu(xMenu);
if ( menu() )
{
Fl_Menu_Item* mi; // non-const pointer
mi = (Fl_Menu_Item*)menu();
int sz = mi->size();
for ( int j=0; j<sz; j++ )
{
// cout << "set: " << j << endl;
color(LBG, LSC);
mi->labelsize(FSIZE);
mi->labelfont(LFONT);
mi->labelcolor(LFG);
mi = mi->next();
}
}
}

//cccccccccccccccccccccccccc
Sbut::~Sbut ()
{
// delete this;
}

//***********************
int main ()
{
Sbut* sb;
WIN = new Fl_Window(0, 0, 200, 300);
G1 = new Fl_Pack(0, 0, 150, 150);
G1->begin();
for ( int i=0; i<5; i++ )
{
sb = new Sbut(i);
idx++;
}
G1->end();
G1->child(0)->label("X");
WIN->end();
WIN->resizable(G1);
WIN->show();
return Fl::run();
}
Edzard Egberts
2013-04-03 06:59:57 UTC
Permalink
Post by marty moore
3. When the add menu item is clicked, a new button appears, but has garbage text.
void set( string str ) { label(str.c_str()); }
label() doesn't copy the string (but there is a copy_label()), but just
saves the given const char*. But your string is local, so the content
vanishs fast. There are several solutions, most easy seems to be:

void set( string str)
{
delete[] label();
copy_label(str.c_str());
}

or a local string as class member:

void set( string str)
{
my_Label= str;
label(my_Label.c_str());
}
MacArthur, Ian (Selex ES, UK)
2013-04-03 07:53:59 UTC
Permalink
Yes, as Edzard says: setting the label of the widget with label() does not *copy* the string, it only retains a pointer to it... which in your case then drops out of scope and garbage ensues.

I think I'd go with the copy_label() approach myself (which *does* make an internal copy of the string), and just let the widget manage the label string internally thereafter...




Selex ES Ltd
Registered Office: Sigma House, Christopher Martin Road, Basildon, Essex SS14 3EL
A company registered in England & Wales. Company no. 02426132
********************************************************************
This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender.
You should not copy it or use it for any purpose nor disclose or
distribute its contents to any other person.
********************************************************************
Albrecht Schlosser
2013-04-06 14:32:15 UTC
Permalink
Post by Edzard Egberts
Post by marty moore
3. When the add menu item is clicked, a new button appears, but has garbage text.
void set( string str ) { label(str.c_str()); }
label() doesn't copy the string (but there is a copy_label()), but just
saves the given const char*. But your string is local, so the content
vanishs fast.
True.
Post by Edzard Egberts
void set( string str)
{
delete[] label();
Please *DON'T* do this (above). The widget "knows better" if it
had copied a label before (with copy_label()) or not (either the
NULL pointer or a pointer assigned with label()). And, you don't
know how the copied string was allocated (in fact it's done with
strdup(), so delete[] is wrong), and the following copy_label()
call would again call free() since you can't reset the internal
pointer and flag.
Post by Edzard Egberts
copy_label(str.c_str());
This is just enough! Let the widget do the rest!

So there is no need to use an own method, unless you want to use
the string type and not use copy_label(str.c_str()) in the program
code directly.
--
Albrecht
marty moore
2013-04-03 14:59:12 UTC
Permalink
Yes, as Edzard says: setting the label of the widget with label() does not =
*copy* the string, it only retains a pointer to it... which in your case th=
en drops out of scope and garbage ensues.
I think I'd go with the copy_label() approach myself (which *does* make an =
internal copy of the string), and just let the widget manage the label stri=
ng internally thereafter...
Selex ES Ltd
Registered Office: Sigma House, Christopher Martin Road, Basildon, Essex SS=
14 3EL
A company registered in England & Wales. Company no. 02426132
********************************************************************
This email and any attachments are confidential to the intended
recipient and may also be privileged. If you are not the intended
recipient please delete it from your system and notify the sender.
You should not copy it or use it for any purpose nor disclose or
distribute its contents to any other person.
********************************************************************
OK, now I understand. It works properly now. Thanks for the help.
Marty
Loading...