On 01/12/12 09:08, Bernhard Schönhammer wrote: > I am just looking into the great Fl_OpDesk from Greg. > Just one question for now: > In Fl_OpDesk::Connect the line 542 reads: > if ( _ConnectOnly(srcbut, dstbut, errmsg) < 0 ) return(-1); > I think it should be "!= 0", because ConnectOnly returns 1, when already > connected.
Hi Bernhard, Thanks for the feedback and bug report. I think the correct solution would be to change that line to read: switch ( _ConnectOnly(srcbut, dstbut, errmsg) ) { case -1: return(-1); // ERROR case 1: return(1); // ALREADY CONNECTED case 0: break; // SUCCESS } This is because both _ConnectOnly() and Connect() are both documented to return a tristate value of 0, 1, or -1, respectively: Success, Already Connected, or Error. So I don't think just != 0 is a good solution, as then Connect() would return -1 instead of 1 for 'already connected'. If you made the != 0 change, and also changed the return(-1) to return(1), that still be bad if _ConnectOnly() returned a -1. Currently _ConnectOnly()'s actual implementation doesn't have code that returns -1, but the docs say it can; this is so that in the future, code might be added that detects errors. We have to trust the docs over the code. So I think the above switch() is the best solution; it ensures the tristate return code is preserved. I'll make that change and test.. should be in the next release of Fl_OpDesk. _______________________________________________ fltk mailing list fltk@easysw.com http://lists.easysw.com/mailman/listinfo/fltk