The problem with different compilation/deployment versions is that the Msg * struct could have changed. So the box in question won't be able to communicate with bearerbox.
== Rene -----Original Message----- From: Nikos Balkanas [mailto:[email protected]] Sent: Monday, 02 August, 2010 20:11 To: Rene Kluwen; 'Alexander Malysh' Cc: 'Kannel Devel' Subject: Re: Patch: EMI UUCP DLRs (final) I see. Isn't it always in openSMPPbox installation instructions to "download and install latest kanell sources"? It wouldn't be a bad idea to release an openSMPPbox patch just to align it with latest kannel. Worst case scenario, one could always make latest kannel sources, without deploying them, just to feed openSMPPbox compilation. The bad thing with these defines, is that they keep accumulating, even after they are useless, out of fear not to break any existing old installations. I think that any client using the gateway's libraries, should follow suit quickly and align their sources, without imposing changes to the gateway. What about SQLbox? Does it also use dlr_find? BR, Nikos ----- Original Message ----- From: "Rene Kluwen" <[email protected]> To: "'Nikos Balkanas'" <[email protected]>; "'Alexander Malysh'" <[email protected]> Cc: "'Kannel Devel'" <[email protected]> Sent: Monday, August 02, 2010 8:43 PM Subject: RE: Patch: EMI UUCP DLRs (final) > The problem is not adding an extra actual parameter. > The problem is that people must now update to the latest Kannel code or > otherwise smppbox won't compile. > > == Rene > > -----Original Message----- > From: Nikos Balkanas [mailto:[email protected]] > Sent: Monday, 02 August, 2010 19:29 > To: Rene Kluwen; 'Alexander Malysh' > Cc: 'Kannel Devel' > Subject: Re: Patch: EMI UUCP DLRs (final) > > Hi Rene, > > Why get things unnecessarily complicated? You can update openSMPPbox code > and leave the new argument always to 0 if you don't need it, or - even > better - set it to 1 to match against EMI and CMDI DLRs. You can always > get > those in openSMPPbox, since bb can use differential routing. > > BR, > Nikos > ----- Original Message ----- > From: "Rene Kluwen" <[email protected]> > To: "'Alexander Malysh'" <[email protected]>; "'Nikos Balkanas'" > <[email protected]> > Cc: "'Kannel Devel'" <[email protected]> > Sent: Monday, August 02, 2010 2:39 AM > Subject: RE: Patch: EMI UUCP DLRs (final) > > >>I propose the following patch. It adds >> >> #define DLR_FIND_USE_DST >> >> To dlr.h. >> >> It is a convenient way to know if I should add an extra parameter to >> dlr_find or not. So smppbox (and possibly other gwlib based programs) can >> happily compile with the dlr_find with the extra option and also (if the >> define does not exist) with the "old" dlr_find. >> >> == Rene >> >> >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] On >> Behalf >> Of Alexander Malysh >> Sent: Wednesday, 28 July, 2010 22:59 >> To: Nikos Balkanas >> Cc: Kannel Devel >> Subject: Re: Patch: EMI UUCP DLRs (final) >> >> Hi, >> >> patch with minor fixes commited to SVN. >> >> Thanks, >> Alexander Malysh >> >> Am 26.07.2010 um 18:14 schrieb Nikos Balkanas: >> >>> Kyriaco, >>> >>> Previous svn diff I got with -ub, which may be relevant. Try this one >> which is only -u. >>> >>> BR, >>> Nikos >>> ----- Original Message ----- From: "Kyriacos Sakkas" >> <[email protected]> >>> To: "Nikos Balkanas" <[email protected]> >>> Cc: "Kannel Devel" <[email protected]> >>> Sent: Monday, July 26, 2010 5:04 PM >>> Subject: Re: Patch: EMI UUCP DLRs (final) >>> >>> >>>> Hi Nikos, >>>> >>>> I got the svn snapshot today. >>>> >>>> Don't think its an SVN problem, possibly some flag needed for patch. >>>> patch 2.5.9 >>>> svn, version 1.4.2 (r22196) >>>> Running on Debian Etch. >>>> >>>> I should be able to hand patch the files where it is giving errors. >>>> Some >>>> from what I saw were due to different indentation. >>>> >>>> If you wish I can post the files detailing the rejected patches. >>>> >>>> Kyriacos >>>> >>>> >>>> On 26/07/2010 16:50, Nikos Balkanas wrote: >>>>> Sorry for the irrelevant question. I tried from a fresh downloaded >>>>> svn: >>>>> >>>>> zdev:~/work/kannel/gateway-> patch < kannel.diff >>>>> Looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> The next patch looks like a unified context diff. >>>>> done >>>>> >>>>> This is from Solaris using sunfreeware's gnu svn: >>>>> >>>>> svn, version 1.6.9 (r901367) >>>>> compiled Mar 20 2010, 16:02:58 >>>>> >>>>> Copyright (C) 2000-2009 CollabNet. >>>>> Subversion is open source software, see http://subversion.tigris.org/ >>>>> This product includes software developed by CollabNet >>>>> (http://www.Collab.Net/). >>>>> >>>>> The following repository access (RA) modules are available: >>>>> >>>>> * ra_neon : Module for accessing a repository via WebDAV protocol >>>>> using Neon. >>>>> - handles 'http' scheme >>>>> - handles 'https' scheme >>>>> * ra_svn : Module for accessing a repository using the svn network >>>>> protocol. >>>>> - with Cyrus SASL authentication >>>>> - handles 'svn' scheme >>>>> * ra_local : Module for accessing a repository on local disk. >>>>> - handles 'file' scheme >>>>> * ra_serf : Module for accessing a repository via WebDAV protocol >>>>> using serf. >>>>> - handles 'http' scheme >>>>> - handles 'https' scheme >>>>> >>>>> Hope this helps, >>>>> Nikos >>>>> ----- Original Message ----- From: "Nikos Balkanas" >> <[email protected]> >>>>> To: "Kyriacos Sakkas" <[email protected]>; "Kannel Devel" >>>>> <[email protected]> >>>>> Sent: Monday, July 26, 2010 4:04 PM >>>>> Subject: Re: Patch: EMI UUCP DLRs (final) >>>>> >>>>> >>>>>> Hi Kyriaco, >>>>>> >>>>>> Are you patching against latest svn? >>>>>> >>>>>> BR, >>>>>> Nikos >>>>>> ----- Original Message ----- From: "Kyriacos Sakkas" >>>>>> <[email protected]> >>>>>> To: "Kannel Devel" <[email protected]> >>>>>> Sent: Monday, July 26, 2010 2:08 PM >>>>>> Subject: Re: Patch: EMI UUCP DLRs (final) >>>>>> >>>>>> >>>>>>> Trying to patch to svn downloaded today (same version as in diff) >>>>>>> and >>>>>>> getting some errors. I will try to change the diff to work, but id >>>>>>> someone has already done this, please forward, >>>>>>> or let me know whats wrong with my patch command line. Patch file is >>>>>>> from Nikos on the 20th. >>>>>>> >>>>>>> /opt/svn/26072010/trunk# patch --dry-run -p0 -b < cimd_dlr.diff >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr_mysql.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr_oracle.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr_pgsql.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr_mem.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr_mssql.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr_sdb.c >>>>>>> Hunk #1 FAILED at 212. >>>>>>> Hunk #2 FAILED at 256. >>>>>>> Hunk #3 FAILED at 285. >>>>>>> 3 out of 5 hunks FAILED -- saving rejects to file gw/dlr_sdb.c.rej >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr.c >>>>>>> Hunk #1 FAILED at 371. >>>>>>> 1 out of 4 hunks FAILED -- saving rejects to file gw/dlr.c.rej >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr.h >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/dlr_p.h >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/smsc/smsc_cimd2.c >>>>>>> Hunk #1 FAILED at 2114. >>>>>>> 1 out of 1 hunk FAILED -- saving rejects to file >>>>>>> gw/smsc/smsc_cimd2.c.rej >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/smsc/smsc_soap.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/smsc/smsc_oisd.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/smsc/smsc_at.c >>>>>>> Hunk #1 succeeded at 2124 with fuzz 1. >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/smsc/smsc_fake.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/smsc/smsc_http.c >>>>>>> Hunk #2 succeeded at 931 with fuzz 1. >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/smsc/smsc_smpp.c >>>>>>> (Stripping trailing CRs from patch.) >>>>>>> patching file gw/smsc/smsc_cgw.c >>>>>>> >>>>>>> >>>>>>> On 26/07/2010 12:40, Kyriacos Sakkas wrote: >>>>>>>> Hi All, >>>>>>>> >>>>>>>> Was away last week too :). >>>>>>>> >>>>>>>> Is this patch already committed to CVS? If not I will patch >>>>>>>> locally. >>>>>>>> Should be able to start running it tomorrow. >>>>>>>> >>>>>>>> Kyriacos >>>>>>>> >>>>>>>> On 22/07/2010 10:51, Byron Kiourtzoglou wrote: >>>>>>>> >>>>>>>>> Hello Niko, >>>>>>>>> >>>>>>>>> I was away for a short vacation. I will try the patch as soon as >>>>>>>>> possible (but >>>>>>>>> I won't be able to test it until mid next week) >>>>>>>>> >>>>>>>>> BRs >>>>>>>>> >>>>>>>>> Byron >>>>>>>>> >>>>>>>>> On Wednesday 21 July 2010 00:47, Nikos Balkanas wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>>> Hi Byrona & Kyriaco, >>>>>>>>>> >>>>>>>>>> Did you get the chance to test the patch? The final one submitted >> is >>>>>>>>>> identical in function, except for some cosmetic changes. I am >> mostly >>>>>>>>>> concerned about the correct behaviour of the concat statement in >>>>>>>>>> Mysql & >>>>>>>>>> Oracle, and of course the overall compatibility with EMI & CIMD2, >>>>>>>>>> and SMPP >>>>>>>>>> for positive control. >>>>>>>>>> >>>>>>>>>> BR, >>>>>>>>>> Nikos >>>>>>>>>> ----- Original Message ----- >>>>>>>>>> From: "Alexander Malysh" <[email protected]> >>>>>>>>>> To: "Nikos Balkanas" <[email protected]> >>>>>>>>>> Cc: <[email protected]> >>>>>>>>>> Sent: Wednesday, July 21, 2010 12:05 AM >>>>>>>>>> Subject: Re: Patch: EMI UUCP DLRs (final) >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Hi Nikos, >>>>>>>>>> >>>>>>>>>> +1 except some indents issues but I will fix it myself... >>>>>>>>>> >>>>>>>>>> Any test results from users? >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> Alexander Malysh >>>>>>>>>> >>>>>>>>>> Am 20.07.2010 um 12:07 schrieb Nikos Balkanas: >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> OK. I believe this is it. >>>>>>>>>>> >>>>>>>>>>> BR, >>>>>>>>>>> Nikos >>>>>>>>>>> ----- Original Message ----- From: "Alexander Malysh" >>>>>>>>>>> <[email protected]> To: "Nikos Balkanas" <[email protected]> >>>>>>>>>>> Cc: "Byron Kiourtzoglou" <[email protected]>; <[email protected]> >>>>>>>>>>> Sent: Tuesday, July 20, 2010 12:14 PM >>>>>>>>>>> Subject: Re: Patch: EMI UUCP DLRs (final) >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Hi Nikos, >>>>>>>>>>> >>>>>>>>>>> see below ... >>>>>>>>>>> >>>>>>>>>>> Am 20.07.2010 um 03:03 schrieb Nikos Balkanas: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>> Hi Alex, >>>>>>>>>>>> >>>>>>>>>>>> Please see inlined comments: I am still waiting feedback from >>>>>>>>>>>> some users >>>>>>>>>>>> testing it. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> ----- Original Message ----- From: "Alexander Malysh" >>>>>>>>>>>>> <[email protected]> >>>>>>>>>>>>> To: "Nikos Balkanas" <[email protected]> >>>>>>>>>>>>> Cc: "Byron Kiourtzoglou" <[email protected]>; >>>>>>>>>>>>> <[email protected]> >>>>>>>>>>>>> Sent: Tuesday, July 20, 2010 1:14 AM >>>>>>>>>>>>> Subject: Re: Patch: EMI UUCP DLRs (final) >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Hi Nikos, >>>>>>>>>>>>> >>>>>>>>>>>>> I'm back from vacation and here are comments to your patch. >>>>>>>>>>>>> Patch looks >>>>>>>>>>>>> OK but still some things to fix: >>>>>>>>>>>>> >>>>>>>>>>>>> --- gw/dlr_mem.c (revision 4833) >>>>>>>>>>>>> +++ gw/dlr_mem.c (working copy) >>>>>>>>>>>>> @@ -125,8 +125,28 @@ >>>>>>>>>>>>> /* XXX: check destination addr too, because e.g. for UCP is >>>>>>>>>>>>> not enough >>>>>>>>>>>>> to check only >>>>>>>>>>>>> * smsc and timestamp (timestamp is even without >>>>>>>>>>>>> milliseconds) >>>>>>>>>>>>> */ >>>>>>>>>>>>> - if(octstr_compare(dlr->smsc,smsc) == 0 && >>>>>>>>>>>>> octstr_compare(dlr->timestamp,ts) == 0) >>>>>>>>>>>>> + if (dst){ >>>>>>>>>>>>> + Octstr *dst_min; >>>>>>>>>>>>> + int len1 = octstr_len(dlr->destination), len2 = >>>>>>>>>>>>> octstr_len(dst); + >>>>>>>>>>>>> + if (len1 < len2) >>>>>>>>>>>>> + return(1); >>>>>>>>>>>>> + >>>>>>>>>>>>> + dst_min = octstr_duplicate(dlr->destination); >>>>>>>>>>>>> + if (len1 > len2) >>>>>>>>>>>>> + octstr_delete(dst_min, 0, len1 - len2); >>>>>>>>>>>>> + >>>>>>>>>>>>> + if (octstr_compare(dlr->smsc, smsc) == 0 && >>>>>>>>>>>>> octstr_compare(dlr-> + timestamp, ts) == 0 && >>>>>>>>>>>>> octstr_compare(dst_min, dst) == 0) { + >>>>>>>>>>>>> octstr_destroy(dst_min); >>>>>>>>>>>>> return 0; >>>>>>>>>>>>> + } >>>>>>>>>>>>> + octstr_destroy(dst_min); >>>>>>>>>>>>> + } >>>>>>>>>>>>> + else >>>>>>>>>>>>> + if (octstr_compare(dlr->smsc, smsc) == 0 && >>>>>>>>>>>>> octstr_compare(dlr-> + timestamp, ts) == 0) >>>>>>>>>>>>> + return 0; >>>>>>>>>>>>> >>>>>>>>>>>>> why so complicated? >>>>>>>>>>>>> if (dst) { >>>>>>>>>>>>> pos = octstr_len(dlr->destination) - octstr_len(dst); >>>>>>>>>>>>> if(pos < 0) >>>>>>>>>>>>> pos = 0; >>>>>>>>>>>>> if (octstr_compare(dlr->smsc, smsc) == 0 && >>>>>>>>>>>>> octstr_compare(dlr->timestamp, ts) == 0 && >>>>>>>>>>>>> octstr_search(dlr->destination, dst, pos) !> = -1) >>>>>>>>>>>>> return 0; >>>>>>>>>>>>> } else ... >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> Same degree of complexity. You use octstr_search, I use >>>>>>>>>>>> octstr_compare. >>>>>>>>>>>> Personally I wouldn't use either, but octstr_search is slightly >>>>>>>>>>>> faster. >>>>>>>>>>>> The only reason I used octstr_delete & compare is because you >>>>>>>>>>>> told me to >>>>>>>>>>>> use truncate on a copy, and I even asked you twice about it. >>>>>>>>>>>> Will change >>>>>>>>>>>> it. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> It's not the same. In my version you don't need duplicate, >>>>>>>>>>> destroy etc. >>>>>>>>>>> and it just simpler to read... >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>>> --- gw/dlr_sdb.c (revision 4833) >>>>>>>>>>>>> +++ gw/dlr_sdb.c (working copy) >>>>>>>>>>>>> + if (dst) >>>>>>>>>>>>> + like = octstr_format("AND \"%S\" LIKE '%%%S' %s", >>>>>>>>>>>>> fields->field_dst, >>>>>>>>>>>>> + dst, sdb_get_limit_str()); >>>>>>>>>>>>> + else >>>>>>>>>>>>> + like = octstr_imm(sdb_get_limit_str()); >>>>>>>>>>>>> >>>>>>>>>>>>> why not keep sdb_get_limit() in place and do it as for other >> DBs? >>>>>>>>>>>>> and \"%S... seems to be wrong... >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> I don't understand what you are asking for here. The code was >>>>>>>>>>>> like that >>>>>>>>>>>> before and I didn't change it. \"%S\" is a typo. Will fix. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> why did you put sdb_get_limit_str() here? it should be keep in >>>>>>>>>>> sql = >>>>>>>>>>> octstr_format("select ... %s", sdb_get_limit_str, ...)... >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>>> --- gw/dlr.c (revision 4833) >>>>>>>>>>>>> +++ gw/dlr.c (working copy) >>>>>>>>>>>>> Msg *msg = NULL; >>>>>>>>>>>>> >>>>>>>>>>>>> struct dlr_entry *dlr = NULL; >>>>>>>>>>>>> >>>>>>>>>>>>> + Octstr *dst_min = NULL; >>>>>>>>>>>>> >>>>>>>>>>>>> indents.... >>>>>>>>>>>>> >>>>>>>>>>>>> + if (use_dst && dst) { >>>>>>>>>>>>> + dst_min = octstr_duplicate(dst); >>>>>>>>>>>>> + int len = octstr_len(dst); >>>>>>>>>>>>> + >>>>>>>>>>>>> + if (len > MIN_DST_LEN) >>>>>>>>>>>>> + octstr_delete(dst_min, 0, len - MIN_DST_LEN); >>>>>>>>>>>>> + } >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> I see. Intends are 3 spaces, instead of 4. That's why I don't >>>>>>>>>>>> use >>>>>>>>>>>> spaces. I tried my best, but please feel free to adjust them >>>>>>>>>>>> exactly the >>>>>>>>>>>> way you like them. >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> :) kannel uses 4 spaces instead of tabs... >>>>>>>>>>> : >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>>>> indents... >>>>>>>>>>>>> >>>>>>>>>>>>> /* destroy struct dlr_entry */ >>>>>>>>>>>>> dlr_entry_destroy(dlr); >>>>>>>>>>>>> + octstr_destroy(dst_min); >>>>>>>>>>>>> >>>>>>>>>>>>> ditto >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>> BR, >>>>>>>>>>>> Nikos >>>>>>>>>>>> --- gw/dlr.h (revision 4833) >>>>>>>>>>>> +++ gw/dlr.h (working copy) >>>>>>>>>>>> +Msg* dlr_find(const Octstr *smsc, const Octstr *ts, const >>>>>>>>>>>> Octstr *dst, >>>>>>>>>>>> int type, >>>>>>>>>>>> + int use_dst); >>>>>>>>>>>> >>>>>>>>>>>> change to >>>>>>>>>>>> +Msg* dlr_find(const Octstr *smsc, const Octstr *ts, const >>>>>>>>>>>> Octstr *dst, >>>>>>>>>>>> int type, int use_dst); >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> Thanks, >>>>>>>>>>>> Alexander Malysh >>>>>>>>>>>> >>>>>>>>>>>> Am 14.07.2010 um 14:38 schrieb Nikos Balkanas: >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>>> Here it is. Added support for dest also for cimd2 as reported >>>>>>>>>>>>> by Byron. >>>>>>>>>>>>> >>>>>>>>>>>>> I have tested thoroughly dlr_mem.c, but only compilation for >>>>>>>>>>>>> DBs, since >>>>>>>>>>>>> i have no access to them. >>>>>>>>>>>>> >>>>>>>>>>>>> @Byron: Could you please test patch for CIMD2 against some of >>>>>>>>>>>>> the DBs >>>>>>>>>>>>> you use? >>>>>>>>>>>>> >>>>>>>>>>>>> Thanks, >>>>>>>>>>>>> Nikos >>>>>>>>>>>>> ----- Original Message ----- From: "Alexander Malysh" >>>>>>>>>>>>> <[email protected]> >>>>>>>>>>>>> To: "Nikos Balkanas" <[email protected]> >>>>>>>>>>>>> Cc: <[email protected]> >>>>>>>>>>>>> Sent: Wednesday, July 07, 2010 6:11 PM >>>>>>>>>>>>> Subject: Re: Patch: EMI UUCP DLRs (final) >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> Hi, >>>>>>>>>>>>> >>>>>>>>>>>>> sorry for delay... I have limited inet access now... see >>>>>>>>>>>>> answers >>>>>>>>>>>>> bellow. >>>>>>>>>>>>> >>>>>>>>>>>>> Am 30.06.2010 um 19:30 schrieb Nikos Balkanas: >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>> >>>>>>>>>>>>>> Please see inlined answers. >>>>>>>>>>>>>> Thanks for the comments and corrections. Please confirm a few >>>>>>>>>>>>>> remaining choices. >>>>>>>>>>>>>> >>>>>>>>>>>>>> BR, >>>>>>>>>>>>>> Nikos >>>>>>>>>>>>>> ----- Original Message ----- From: "Alexander Malysh" >>>>>>>>>>>>>> <[email protected]> >>>>>>>>>>>>>> To: "Nikos Balkanas" <[email protected]> >>>>>>>>>>>>>> Cc: <[email protected]> >>>>>>>>>>>>>> Sent: Wednesday, June 30, 2010 12:02 PM >>>>>>>>>>>>>> Subject: Re: Patch: EMI UUCP DLRs (final) >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> IMO we don't need to handle no destination case in DLR >>>>>>>>>>>>>>> lookup but >>>>>>>>>>>>>>> maybe it's not a wrong idea to be able to ignore destination >>>>>>>>>>>>>>> for some >>>>>>>>>>>>>>> reasons when SMSC send some junk to us. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> Not doing it, could miss some queries altogether. It would >>>>>>>>>>>>>> still work >>>>>>>>>>>>>> for the large majority, but could miss a few matches that >>>>>>>>>>>>>> would have >>>>>>>>>>>>>> gotten otherwise. >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> this is ok for me, we make it optional... >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>>> ok here is the review for your patch: >>>>>>>>>>>>>>> + if (dst){ >>>>>>>>>>>>>>> + int len = octstr_len(dst); >>>>>>>>>>>>>>> + char *p = octstr_get_cstr(dst); >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> + if (len > MIN_DST_LEN) >>>>>>>>>>>>>>> + p += len - MIN_DST_LEN; /* get last >>>>>>>>>>>>>>> MIN_DST_LEN >>>>>>>>>>>>>>> digits */ >>>>>>>>>>>>>>> + like = octstr_create(strcat("%", p)); >>>>>>>>>>>>>>> + gwlist_append(binds, like); >>>>>>>>>>>>>>> + } >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> why is this in every dlr implementation? we have abstraction >>>>>>>>>>>>>>> for >>>>>>>>>>>>>>> this, see dlr.c >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> I don't know where in dlr.c you are referring, but I see your >>>>>>>>>>>>>> reasoning. >>>>>>>>>>>>>> This is leftover from a previous implementation, where I >>>>>>>>>>>>>> passed >>>>>>>>>>>>>> use_dest in the dlr_<DB>. I considered it at the time >>>>>>>>>>>>>> important to >>>>>>>>>>>>>> pass the whole dest to dlr_<db> so that debug messages get >>>>>>>>>>>>>> the whole >>>>>>>>>>>>>> dest. Since I started passing NULL for dest, this serves no >> more >>>>>>>>>>>>>> purpose. I will abstract it in dlr_find and use octstr_delete >>>>>>>>>>>>>> instead. >>>>>>>>>>>>>> Will also correct debug messages accordingly in dlr_db.c. >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> you have to abstract it in dlr_find and not repeat the same >>>>>>>>>>>>> code in >>>>>>>>>>>>> each dlr_db driver. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>>> - sql = octstr_format("SELECT `%S`, `%S`, `%S`, `%S`, >>>>>>>>>>>>>>> `%S`, `%S` >>>>>>>>>>>>>>> FROM `%S` WHERE `%S`=? AND `%S`=? LIMIT 1", >>>>>>>>>>>>>>> + if (dst) >>>>>>>>>>>>>>> + sql = octstr_format("SELECT `%S`, `%S`, `%S`, `%S`, >>>>>>>>>>>>>>> `%S`, >>>>>>>>>>>>>>> `%S` FROM `%S` WHERE `%S`=? AND `%S`=? AND `%S` >LIKE ? >>>>>>>>>>>>>>> LIMIT 1", >>>>>>>>>>>>>>> fields->field_mask, fields->field_serv, >>>>>>>>>>>>>>> fields->field_url, fields->field_src, >>>>>>>>>>>>>>> fields->field_dst, fields->field_boxc, >>>>>>>>>>>>>>> fields->table, fields->field_smsc, >>>>>>>>>>>>>>> + fields->field_ts, >>>>>>>>>>>>>>> fields->field_dst); >>>>>>>>>>>>>>> ... >>>>>>>>>>>>>>> First of all: like ? doesn't work as you expect... it should >> be >>>>>>>>>>>>>>> something like: LIKE CONCAT('%%', ?) and >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> Thanks. I will look into it. >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>>> this is too much maintenance for SQL that defined two times, >>>>>>>>>>>>>>> how >>>>>>>>>>>>>>> about like this: >>>>>>>>>>>>>>> if (dst) >>>>>>>>>>>>>>> like = octstr_format('LIKE CONCAT('%%', ?)' ...); >>>>>>>>>>>>>>> else >>>>>>>>>>>>>>> like = octstr_create(""); >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> You probably mean: >>>>>>>>>>>>>> >>>>>>>>>>>>>> like = octstr_create("=?"); >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> no, I mean octstr_create("") or better use octstr_imm("") >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>>> sql = ...(".... %S", like) >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> This is a classical maintenance vs overhead. Malloc is >>>>>>>>>>>>>> expensive, much >>>>>>>>>>>>>> more so in Linux than in Solaris. Furthermore, sql="%s%S" is >>>>>>>>>>>>>> more >>>>>>>>>>>>>> difficult to read and understand, since SQL mechanism is not >>>>>>>>>>>>>> explicit, >>>>>>>>>>>>>> but hidden in variables. Do you really want that? >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> yes, because maintenance is then easier and malloc overhead is >>>>>>>>>>>>> in linux >>>>>>>>>>>>> not so much expensive as you think because glibc has >> preallocated >>>>>>>>>>>>> memory pools and not always is system call >>>>>>>>>>>>> needed. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>>> The same is for DELETE, UPDATE... >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> + if (like) octstr_destroy(like); >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> octstr_destroy will check for NULL for you... >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> I am aware of that, but it costs a function call and a few >>>>>>>>>>>>>> more >>>>>>>>>>>>>> statements...Either way is fine. I can change it. >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> function call is not really issue against code readability... >>>>>>>>>>>>> and if >>>>>>>>>>>>> this is really your argument then convert all calls to if >>>>>>>>>>>>> (..!=NULL) >>>>>>>>>>>>> bla... (just a joke) >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>>> + if (octstr_compare(dlr->smsc, smsc) == 0 && >>>>>>>>>>>>>>> octstr_compare(dlr-> >>>>>>>>>>>>>>> + timestamp, ts) == 0 && memcmp(p1 + len1 - size, >>>>>>>>>>>>>>> p2 + >>>>>>>>>>>>>>> len2 - size, >>>>>>>>>>>>>>> + size) == 0) >>>>>>>>>>>>>>> + return 0; >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> memcmp??? why not just use truncated destination and do: >>>>>>>>>>>>>>> octstr_search??? >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> Actually octstr_truncate won't work since it truncates from >>>>>>>>>>>>>> the end. >>>>>>>>>>>>>> octstr_delete, would work, however, destroying the original >>>>>>>>>>>>>> Octstr in >>>>>>>>>>>>>> the process, unless I duplicated them. It would need to be >>>>>>>>>>>>>> done on >>>>>>>>>>>>>> both destinations to work. Code would be more, and the >>>>>>>>>>>>>> malloc, free >>>>>>>>>>>>>> and copy, have an overhead. Memcmp doesn't change the >>>>>>>>>>>>>> original Octstr >>>>>>>>>>>>>> and is natural for such operations. However, it is out of >> kannel >>>>>>>>>>>>>> style, so you have every right to ask me to change it. Do >>>>>>>>>>>>>> you? >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> yes, please change it. >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>>> +Msg *dlr_find(const Octstr *smsc, const Octstr *ts, const >>>>>>>>>>>>>>> Octstr >>>>>>>>>>>>>>> *dst, int typ, int use_dst) >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> you don't need to change function. Just use dst = NULL and >>>>>>>>>>>>>>> check it. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>>> No. dst is needed for debug msgs inside dlr_find. >>>>>>>>>>>>>> Furthermore, >>>>>>>>>>>>>> use_dst, currently is set only for EMI. Decision is made at >>>>>>>>>>>>>> driver >>>>>>>>>>>>>> level so it can easily change to an smsc configuration >>>>>>>>>>>>>> variable if >>>>>>>>>>>>>> needed. >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>> ok, maybe you are right. What other people think about this ? >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>>>>> Am 26.06.2010 um 10:23 schrieb Nikos Balkanas: >>>>>>>>>>>>>> >>>>>>>>>>>>>> >>>>>>>>>>>>>>> Hi, >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> I believe I have accounted for almost all your comments to >>>>>>>>>>>>>>> produce >>>>>>>>>>>>>>> the final working patch. I have no way of testing, other >>>>>>>>>>>>>>> than >>>>>>>>>>>>>>> compilation. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> This patch will povide: >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> 1) Align dlr_oracle.c with the rest. Currently, Oracle does >>>>>>>>>>>>>>> a full >>>>>>>>>>>>>>> dst find/remove/update on each DLR. >>>>>>>>>>>>>>> 2) Dst use on find/remove/update is controlled in dlr_find >>>>>>>>>>>>>>> by >> a >>>>>>>>>>>>>>> single variable, use_dst. This is currently set only at >>>>>>>>>>>>>>> driver level >>>>>>>>>>>>>>> and only in the emi driver, while everyone else has it >>>>>>>>>>>>>>> false. >>>>>>>>>>>>>>> However, very easily, if need arises, it can be set in smsc >>>>>>>>>>>>>>> configuration. 3) All DLR handling for all smscs will remain >>>>>>>>>>>>>>> as it >>>>>>>>>>>>>>> used to be till now. Only emi handling changes, hopefully >>>>>>>>>>>>>>> for the >>>>>>>>>>>>>>> better (that is the purpose of the patch :-)). >>>>>>>>>>>>>>> 4) It will try to match the last 7 digits of the destination >>>>>>>>>>>>>>> or the >>>>>>>>>>>>>>> length of the destination if it smaller. This is defined in >>>>>>>>>>>>>>> gw/dlr_p.h as MIN_DST_LEN. Didn't want to make it larger, >>>>>>>>>>>>>>> wanted to >>>>>>>>>>>>>>> avoid prefix territory at all costs, since I have seen a lot >> of >>>>>>>>>>>>>>> mangling there by the SMScs. Besides, I believe that 7 >>>>>>>>>>>>>>> digits give >>>>>>>>>>>>>>> enough resolution 5) People using emi, should rebuild their >>>>>>>>>>>>>>> indeces, >>>>>>>>>>>>>>> especially if they are running large batch jobs through EMI. >>>>>>>>>>>>>>> The LIKE >>>>>>>>>>>>>>> % construction is not very efficient. >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> Enjoy, >>>>>>>>>>>>>>> Nikos >>>>>>>>>>>>>>> <kannel.diff> >>>>>>>>>>>>>>> >>>>>>>>>>>>>>> >>>>>>>>>>>>> <kannel.diff> >>>>>>>>>>>>> >>>>>>>>>>>>> >>>>>>>>>>> <kannel.diff> >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> -- >>>>>>> Kyriacos Sakkas >>>>>>> Development Team >>>>>>> Netsmart >>>>>>> Tel: + 357 22 452565 >>>>>>> Fax: + 357 22 452566 >>>>>>> Email: [email protected] >>>>>>> http://www.netsmart.com.cy >>>>>>> >>>>>>> Taking Business to a New Level! >>>>>>> >>>>>>> ** Confidentiality Notice: The information contained in this email >>>>>>> message may be privileged, confidential and protected from >>>>>>> disclosure. >>>>>>> If you are not the intended recipient, any dissemination, >> distribution, >>>>>>> or copying of this email message is strictly prohibited. >>>>>>> If you think that you have received this email message in error, >> please >>>>>>> email the sender at [email protected] ** >>>>>>> >>>>>> >>>> >>>> >>>> -- >>>> Kyriacos Sakkas >>>> Development Team >>>> Netsmart >>>> Tel: + 357 22 452565 >>>> Fax: + 357 22 452566 >>>> Email: [email protected] >>>> http://www.netsmart.com.cy >>>> >>>> Taking Business to a New Level! >>>> >>>> ** Confidentiality Notice: The information contained in this email >>>> message may be privileged, confidential and protected from disclosure. >>>> If you are not the intended recipient, any dissemination, distribution, >>>> or copying of this email message is strictly prohibited. >>>> If you think that you have received this email message in error, please >>>> email the sender at [email protected] ** >>> <kannel.diff> >> >> >> >> > > >
