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

Reply via email to