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>
dlr_find_define.patch
Description: Binary data
