Re: Proposed Patch for Consideration for Routing.cxx to handle 2 column SQL return better.

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

 



Andrew,

I'm not sure the patch you propose is a good idea. Your code treats a
NULL in the second result column as as if the column wasn't there.

a.) This shadows bugs in the SQL code causing the NULL value, eg. a
missing IFNULL(). If the SQL is written to return 2 columns, there is
good chance that the rest of the configuration relies on a gateway IP
being set. If your calls still connect, thats sheer luck.

b.) We having special treatment for NULL in one case, but not in other
columns and not in other SQL modules (SQLAuth etc.) makes the behavior
somewhat inconsistent.

I think I would prefer a patch that produce warning messages if NULL is
encountered in any column across all SQL modules.

Regards,
Jan


Andrew Herdman wrote:
> Jan and Group;
> 
> I've been playing heavily with the SQL routing policy pieces of recent 
> and stumbled upon an issue with a database structure that returns 2 
> columns, when the second one is NULL.    If this case, the call fails.  
> If the query only returns one column, the function works properly, and 
> if you ensure that a gateway IP in a case where two columns are being 
> returned, those calls work (unless you try to use your own IP for the 
> gateway).
> 
> Anyway, looking at the code, I copied, and edited this patch, it works 
> in all cases (translate just alias, or add gateway IP, or lastly, 
> translate alias and add gateway IP).  I'd like to submit this patch for 
> consideration to be included in the CVS code.  Please excuse the 
> comments, they were put into the code so I could track my changes.
> 
> Thank you
>   Andrew
> 
> --- Routing.cxx 2010-08-27 08:24:08.000000000 -0400
> +++ /root/openh323gk/Routing.cxx  2010-08-26 14:24:49.000000000 -0400
> @@ -1794,7 +1794,21 @@ void SqlPolicy::DatabaseLookup(
>                         H323SetAliasAddress(newDestination, newAliases[0]);
>                         destination.SetNewAliases(newAliases);
>                 }
> -       } else if (result->GetNumFields() == 2) {
> +// Added by Andrew Herdman to get around the NULL 2nd column (2010-08-26)
> +       } else if ((result->GetNumFields() == 2) && (resultRow[1].first 
> == NULL))  {
> +               PString newDestinationAlias = resultRow[0].first;
> +               PString newDestinationIP = resultRow[1].first;
> +               PTRACE(5, m_name << "\tQuery result : " << 
> newDestinationAlias << ", " << newDestinationIP);
> +               if (newDestinationAlias.ToUpper() == "REJECT") {
> +                       destination.SetRejectCall(true);
> +               } else {
> +                       H225_ArrayOf_AliasAddress newAliases;
> +                       newAliases.SetSize(1);
> +                       H323SetAliasAddress(newDestinationAlias, 
> newAliases[0]);
> +                       destination.SetNewAliases(newAliases);
> +               }
> +       } else if ((result->GetNumFields() == 2) && (resultRow[1].first 
> != NULL)) {
> +// End Code Added by Andrew Herdman (2010-08-26)
>                 PString newDestinationAlias = resultRow[0].first;
>                 PString newDestinationIP = resultRow[1].first;
>                 PTRACE(5, m_name << "\tQuery result : " << 
> newDestinationAlias << ", " << newDestinationIP);

-- 
Jan Willamowius, jan@xxxxxxxxxxxxxx, http://www.gnugk.org/

------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users 
worldwide. Take advantage of special opportunities to increase revenue and 
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d
_______________________________________________________

Posting: mailto:Openh323gk-users@xxxxxxxxxxxxxxxxxxxxx
Archive: http://sourceforge.net/mailarchive/forum.php?forum_name=openh323gk-users
Unsubscribe: http://lists.sourceforge.net/lists/listinfo/openh323gk-users
Homepage: http://www.gnugk.org/


[Index of Archives]     [SIP]     [Open H.323]     [Gnu Gatekeeper]     [Asterisk PBX]     [ISDN Cause Codes]     [Yosemite News]

  Powered by Linux