| 00:11.56 | *** join/#asterisk-dev superscrat (~asanders@173-17-133-2.client.mchsi.com) |
| 00:13.07 | *** join/#asterisk-dev babak (uid19622@gateway/web/irccloud.com/x-tsjrrzvzcicllraz) |
| 01:09.31 | *** join/#asterisk-dev superscrat (~asanders@173-17-133-2.client.mchsi.com) |
| 01:13.16 | *** join/#asterisk-dev superscrat (~asanders@173-17-133-2.client.mchsi.com) |
| 01:26.22 | *** join/#asterisk-dev gtjoseph (~gtj0@unaffiliated/gtj) |
| 02:06.36 | *** join/#asterisk-dev leedm777 (~leedm777@50-81-28-152.client.mchsi.com) |
| 03:18.47 | *** join/#asterisk-dev drmessano (~nonya@pdpc/supporter/active/drmessano) |
| 03:53.05 | coreyfarrell | gtjoseph: ping |
| 04:03.18 | *** join/#asterisk-dev tzafrir (~tzafrir@bzq-218-28-58.cablep.bezeqint.net) |
| 04:03.39 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 06:07.46 | *** join/#asterisk-dev wdoekes (~walter@wjd.osso.nl) |
| 06:42.54 | *** join/#asterisk-dev hehol (~hehol@gatekeeper.loca.net) |
| 07:14.27 | *** join/#asterisk-dev tzafrir (~tzafrir@local.xorcom.com) |
| 07:14.27 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 07:19.23 | *** join/#asterisk-dev pchero_work (~pchero@109.70.54.56) |
| 08:34.40 | *** join/#asterisk-dev pchero_work (~pchero@109.70.54.56) |
| 09:19.43 | *** join/#asterisk-dev wanna (~wanna@194.183.244.5) |
| 09:32.24 | *** join/#asterisk-dev pnlarsson (~niklas@fw1.gml.g.icnet.infracom.se) |
| 10:14.56 | *** join/#asterisk-dev y_ateya (~y_ateya@41.45.14.255) |
| 10:15.29 | y_ateya | Hi :) |
| 10:15.56 | y_ateya | When to expect asterisk 13.4? I thought it should be at end of April. |
| 10:17.32 | y_ateya | I tried to work with head of 13 branch, but wasn't stable (some FD leaks due to blond transfer, which was fixed later). |
| 10:18.56 | coreyfarrell | y_ateya: my guess is that all releases got delayed a bit due to the git conversion |
| 10:39.34 | file | the script, and some associated dependencies, are still up for review on gerrit |
| 10:39.42 | file | the script that makes releases - that is |
| 10:49.32 | *** join/#asterisk-dev Tim_Toady (~fuzzy@snf-33276.vm.okeanos.grnet.gr) |
| 11:54.58 | *** join/#asterisk-dev jrose_atDigium (jrose_atDi@nat/digium/x-dtuhtsagxfzvrgpq) |
| 11:57.06 | file | la la la |
| 12:15.38 | *** join/#asterisk-dev Marquis42 (~mrbmw@d14-69-246-193.try.wideopenwest.com) |
| 12:17.01 | *** join/#asterisk-dev [TK]D-Fender (~chatzilla@216-191-106-163.dedicated.allstream.net) |
| 12:43.57 | file | coreyfarrell, I think the ordering change in spandsp caused it to not build anymore |
| 12:52.18 | coreyfarrell | :( it's res_snmp that isn't building. |
| 13:01.56 | file | res_snmp and res_fax_spandsp |
| 13:03.02 | coreyfarrell | https://bamboo.asterisk.org/bamboo/browse/AST-ATRUNKBUILD-BASICBUILD-1844/log shows spandsp building fine? |
| 13:03.54 | file | uno sec |
| 13:04.45 | file | http://pastebin.com/mhLa8jzf |
| 13:06.43 | coreyfarrell | AST_BUILDOPTS? |
| 13:07.53 | file | #define AST_BUILDOPTS "AST_DEVMODE, LOADABLE_MODULES, OPTIONAL_API" |
| 13:09.37 | file | this is on Ubuntu 15.04 |
| 13:10.58 | coreyfarrell | did you have a failure on res_snmp with MALLOC_DEBUG off? or just res_fax_spandsp? |
| 13:11.33 | file | I don't remember a failure for it |
| 13:12.42 | *** join/#asterisk-dev leedm777 (leedm777@nat/digium/x-uzljwuyvjucmgvxe) |
| 13:30.45 | coreyfarrell | file: I have a patch that works for res_fax_spandsp with and without MALLOC_DEBUG. installing libsnmp-dev so I can test res_snmp (I think the fix will work for that too) |
| 13:31.04 | file | k |
| 13:31.48 | file | cool |
| 13:31.50 | coreyfarrell | just have to say I really hate these wrapper macros |
| 13:32.27 | coreyfarrell | for malloc/free/etc.. they cause more trouble then they're worth when we can just review code to ensure certain standard functions are not being used |
| 13:42.44 | hexanol | I asked a question last friday but had to leave before I got an answer, so I'm asking it again |
| 13:42.49 | hexanol | I found in app_queue.c something similar to: ast_json_pack("{s: i}", foo, (long) bar) |
| 13:42.53 | hexanol | is the cast to long suspicious ? |
| 13:42.59 | hexanol | because at other place in app_queue.c, when the 'i' format specifier is used, an explit cast to int is used, not long |
| 14:04.34 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-uozjqojnxuphletd) |
| 14:09.45 | *** join/#asterisk-dev malcolmd (malcolmd@pdpc/sponsor/digium/malcolmd) |
| 14:09.45 | *** mode/#asterisk-dev [+o malcolmd] by ChanServ |
| 14:15.17 | file | hexanol, from mjordan - hexanol: in case you come back or read this in a log, it should be json_int_t (https://jansson.readthedocs.org/en/2.2/apiref.html#json_int_t) |
| 14:23.33 | *** join/#asterisk-dev roramirez (~rodrigo@190.54.225.167) |
| 14:23.42 | roramirez | Hello |
| 14:26.28 | coreyfarrell | file: https://gerrit.asterisk.org/442 for the fixes |
| 14:26.35 | file | kk |
| 14:31.19 | coreyfarrell | file: for the asterisk.conf debug=yes fix is it the first line you have a problem with? I thought I explained what was going on in the rest of the commit msg. |
| 14:32.02 | file | you've said what happens, but not why it's a problem |
| 14:36.16 | coreyfarrell | file: updated https://gerrit.asterisk.org/437 only, if that one is good I'll update 13/master (I have another review for master that comes after this fix) |
| 14:36.37 | file | that's fine |
| 14:38.10 | gtjoseph | coreyfarrell: pong from last night. :) |
| 14:39.03 | roramirez | coreyfarrell: you can help me with a review?, i don't understand a little thing. |
| 14:40.51 | coreyfarrell | gtjoseph: well the vector code is already merged so I'm a little late... I was looking at manager code for the review that roramirez has on, and realized that every case where I run AST_VECTOR_ADD_SORTED will be after I iterate the list to avoid duplicates. I was wondering if there was a way to do a nodups parameter or something to avoid scanning the list twice :) |
| 14:40.55 | coreyfarrell | roramirez: whats up? |
| 14:42.21 | roramirez | coreyfarrell: the review is https://gerrit.asterisk.org/435/ |
| 14:43.06 | coreyfarrell | what about it? |
| 14:44.20 | roramirez | the problem is now is not showing in alphabetically order. |
| 14:45.24 | coreyfarrell | yes, I can confirm that.. but the problem is not on the display side. the list &actions is supposed to be in alphabetical order, something is broken with the insert process |
| 14:45.31 | roramirez | where is the "insert side" ? |
| 14:46.40 | coreyfarrell | in ast_manager_register_struct.. |
| 14:55.22 | roramirez | ok, understand. Thanks you. |
| 14:56.22 | coreyfarrell | roramirez: I have a feeling it's something to do with how prev is assigned, but I don't have time to dig into it. |
| 14:58.11 | gtjoseph | coreyfarrell: you could do a bsearch on the vector |
| 14:58.20 | roramirez | coreyfarrell: yesterday i saw something similar. let me search. |
| 15:00.53 | hexanol | well from the jansson doc, it seems like the 'i' specifier should be used with an int, not a json_int_t (which should be used with a 'I' (i.e. uppercase i) specifier) |
| 15:01.33 | coreyfarrell | gtjoseph: that would still scan the vector twice. once to verify non-existence of the search key, then again to find the position we should put the new element assuming it's not found. |
| 15:01.38 | roramirez | could this be? https://github.com/asterisk/asterisk/commit/ba9867fa |
| 15:02.44 | gtjoseph | coreyfarrell: true. i could add a NODUPS version that rejects if it finds an existing key. |
| 15:04.16 | coreyfarrell | gtjoseph: or we could just add an allowdups param to the ADD_SORT? not sure.. this could also be a low impact optimization that might not be worth doing? |
| 15:05.21 | coreyfarrell | roramirez: not related, that's for modules, not manager actions. |
| 15:05.47 | gtjoseph | thats why i was going to do a separate NODUPS version. It's only a minor hit to check the result of the compare operation for 0 |
| 15:08.10 | roramirez | coreyfarrell: is not the change explicit, is the used of AST_DLLIST*. |
| 15:10.00 | coreyfarrell | gtjoseph: I'm not a fan of the separate NODUPS version for the same reason I didn't like the split AST_VECTOR_REMOVE.. the allowdups would normally be a constant value so the conditional should get optimized appropriately, would just be duplicating a bunch of code. |
| 15:11.15 | gtjoseph | yep. i think that makes it "a low impact optimization that might not be worth doing" then |
| 15:12.30 | gtjoseph | for a small vector, doing a GET_CMP before the INSERT wouldn't be a big impact and for a large vector, do a bsearch. |
| 15:15.09 | coreyfarrell | roramirez: sorry I'm not following you. that change caused modules to not be sorted in the list. this has *nothing* to do with manager actions being out of order. the switch to unsorted DLLIST was to make module unloading go in the reverse of load order. |
| 15:18.17 | *** join/#asterisk-dev rmudgett (rmudgett@nat/digium/x-qvwtrkijokzwifao) |
| 15:20.03 | roramirez | ok coreyfarrell, thanks you. |
| 15:25.31 | *** join/#asterisk-dev ipengineer (~zconkle@static-71-252-134-63.dllstx.fios.verizon.net) |
| 15:30.02 | *** join/#asterisk-dev superscrat (asanders@nat/digium/x-lmilinsqgdffigme) |
| 15:37.17 | gtjoseph | file: i could do wizard remove by name instead of by index since there can;t be dups. this way shifts aren't an issue |
| 15:38.40 | file | or by wizard interface pointer |
| 15:39.17 | gtjoseph | name would probably be friendlier |
| 15:39.50 | file | fine |
| 15:41.14 | gtjoseph | actually &name = &wizard = &internal_wizard as it happens :) |
| 16:01.34 | coreyfarrell | file: when you get a chance can you +1 https://gerrit.asterisk.org/392 again? had to be rebased because of the commit msg change of the other review |
| 16:02.30 | file | I will. |
| 16:02.44 | file | I'm letting others do code reviews. |
| 16:16.59 | *** join/#asterisk-dev sgriepentrog (sgriepentr@nat/digium/x-ovthedaldtvlbuyc) |
| 16:27.43 | gtjoseph | file: trying to test your review 430 and it's failing on minversion 14.0.0. Shouldn't "Asterisk GIT-master-58d0db3" satisfy that? |
| 16:38.45 | *** join/#asterisk-dev leedm777 (leedm777@nat/digium/x-kugwsedeceobzlow) |
| 16:40.05 | *** join/#asterisk-dev pchero (~pchero@0x555140b5.adsl.cybercity.dk) |
| 16:40.10 | file | gtjoseph, it's marked as skip because it's broken in PJSIP |
| 16:40.56 | gtjoseph | ah, ok. the output just says Met: false for version. |
| 16:41.40 | superscrat | coreyfarrell, I responded to your comment, and I can +1 your code review. The code looks fine to me - just one very small point of contention of a quantifier word. because I am a dork :p |
| 16:43.50 | coreyfarrell | superscrat: i'm making the change |
| 16:44.17 | superscrat | cool beans. thanks! |
| 16:45.52 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-smfparxpxtmmavej) |
| 16:47.37 | coreyfarrell | superscrat: updated |
| 16:47.40 | superscrat | coreyfarrel, I gave it a +1. Thank you! |
| 16:48.09 | superscrat | err coreyfarrell, I am so sorry |
| 16:48.28 | superscrat | my key stuck. I am trying to eat delicious mac n cheese and type at the same time. |
| 16:49.34 | coreyfarrell | superscrat: no worries. thanks for helping improve the CHANGES and UPGRADE.txt |
| 16:50.23 | superscrat | hey, no problem =) |
| 16:59.32 | *** join/#asterisk-dev babak (uid19622@gateway/web/irccloud.com/x-gpkfjzjvablpbiul) |
| 17:03.52 | *** join/#asterisk-dev kharwell (kharwell@nat/digium/x-axpfdzbqekpyxvyg) |
| 17:22.57 | *** join/#asterisk-dev Corydon76 (~quassel@c-69-137-93-158.hsd1.tn.comcast.net) |
| 17:22.58 | *** mode/#asterisk-dev [+o Corydon76] by ChanServ |
| 17:51.20 | *** join/#asterisk-dev roramirez (~rodrigo@190.54.225.167) |
| 18:52.17 | coreyfarrell | rmudgett: any suggestions for astmm.h? |
| 18:57.18 | *** join/#asterisk-dev roramirez (~rodrigo@186-78-6-236.baf.movistar.cl) |
| 19:00.54 | rmudgett | I'd say wrap the libc free() and friends by default unless NOWRAP_LIBC_MALLOC is defined. Get rid of the WRAP_LIBC_MALLOC define as it conflicts and won't be necessary anymore. |
| 19:01.24 | rmudgett | Pure Asterisk code should never be using plain free() and friends anyway. |
| 19:04.02 | rmudgett | Though we have lpc10 code that is considered part of asterisk that uses free() and friends that we shouldn't change to using ast_free. |
| 19:05.25 | coreyfarrell | my idea with the patch that's up is that a source should only define NOWRAP or WRAP (or nothing). with NOWRAP libc functions are used directly. with WRAP libc functions are remapped to asterisk functions, and with neither libc functions will cause a compile error. |
| 19:05.34 | rmudgett | So we have 3 cases so far. 1) Pure asterisk that cannot ever use free(). 2) Intermediate like lpc10 that needs to redirect free() to ast_free(). 3) External (spandsp) that cannot redirect free(). |
| 19:06.53 | coreyfarrell | yes.. previously a static inline in spandsp headers would call free(), that would be remapped to Do_not_use_free__use_ast_free_or_ast_std_free_for_remotely_allocated_memory(), and since it was optimized away there would be no error or warning |
| 19:07.07 | rmudgett | Those names WRAP/NOWRAP suggest boolean |
| 19:07.31 | coreyfarrell | what do you mean? |
| 19:08.11 | rmudgett | You have three states wrap/nowrap/nothing but the wrap names suggest boolean |
| 19:08.55 | coreyfarrell | so when I added ->fail to the Do_not_use_.. calls, it caused static inline functions using free to become an error. without that change the use of free() in lpc10/lpcini.c caused no warning but runtime failed to load for the missing symbol |
| 19:09.38 | coreyfarrell | would it be better if I changed code through-out asterisk to #define WRAP_LIBC_MALLOC 1, define to 0 for spandsp/snmp? |
| 19:10.35 | rmudgett | I'm trying to think of a better name to not suggest boolean a state |
| 19:11.12 | file | DAVE! |
| 19:11.12 | coreyfarrell | so for !defined we map to Do_not_use.. for ==0 we allow free to be used direct, for ==1 we replace free with ast_free |
| 19:12.00 | coreyfarrell | DIRECT_LIBC_MALLOC? |
| 19:12.01 | leedm777 | glances at file |
| 19:12.09 | file | leedm777, no - you are DaveOps |
| 19:12.11 | rmudgett | Probably assigning a value to WRAP_LIBC_MALLOC 0< or not-defined>,1<cannot use free>,2<free means free> |
| 19:12.21 | leedm777 | ah; still getting use to that |
| 19:13.09 | rmudgett | Maybe better would be to give 0,1,2 names |
| 19:15.45 | rmudgett | There aren't that many files that define WRAP_LIBC_MALLOC |
| 19:15.55 | coreyfarrell | I'll probably add an #ifndef/#define/#endif block to astmm.h so we can just check for the values without worrying about !defined |
| 19:17.11 | rmudgett | NEVER - cannot use free |
| 19:17.37 | rmudgett | REDIRECT - free redirected to ast_free |
| 19:18.04 | rmudgett | IGNORE - free means free |
| 19:18.40 | rmudgett | Still not happy with those names though |
| 19:18.56 | coreyfarrell | I would like them better if it was for AST_LIBC_MALLOC instead of WRAP |
| 19:19.17 | coreyfarrell | maybe s/NEVER/BLOCK/ |
| 19:20.08 | rmudgett | AST_LIBC_MALLOC is probably better than WRAP_LIBC_MALLOC. |
| 19:21.02 | rmudgett | yeah BLOCK is better |
| 19:22.15 | coreyfarrell | so what about prefix for BLOCK/REDIRECT/IGNORE? seems like we're asking for trouble using a single word with no prefix |
| 19:23.27 | rmudgett | Prefix is fine. Was just trying to get the concept captured in a single word. |
| 19:23.37 | coreyfarrell | another thought, is it ok for sources to have '#define AST_LIBC_MALLOC BLOCK' before BLOCK is defined? |
| 19:24.04 | rmudgett | ASTMM_BLOCK/ASTMM_REDIRECT/ASTMM_IGNORE |
| 19:24.17 | rmudgett | ASTMM_LIBC_MALLOC |
| 19:25.11 | coreyfarrell | or just ASTMM_LIBC? the _MALLOC makes it sound to me like it does less than it does. |
| 19:25.40 | rmudgett | ok |
| 19:26.25 | rmudgett | Defines are not processed by the compiler and the pre processor doesn't care what it is defined as until the value is actually used. |
| 19:26.55 | coreyfarrell | great. last thought, for '#if defined(ASTMM_LIBC) && (ASTMM_LIBC < 0 || ASTMM_LIBC > 2)', #error ? |
| 19:28.23 | rmudgett | That works because the values should never resolve to anything else. |
| 19:28.24 | coreyfarrell | or just use an #if, #elif, #elif to check the accepted values, then #error in the #else block? |
| 19:29.05 | rmudgett | The #if #elif ladder does avoid having to worry about what the actual numeric value is. |
| 19:30.41 | rmudgett | #if !defined(ASTMM_LIBC) then #define ASTMM_LIBC ASTMM_BLOCK |
| 19:31.31 | coreyfarrell | yup already added that :) |
| 19:31.52 | rmudgett | #if (ASTMM_LIBC != ASTMM_BLOCK) && (ASTMM_LIBC != ASTMM_REDIRECT) && (ASTMM_LIBC != ASTMM_IGNORE) then #error |
| 19:32.32 | rmudgett | Before that you have to give ASTMM_BLOCK, ASTMM_REDIRECT, and ASTMM_IGNORE their actual values. |
| 19:33.36 | coreyfarrell | personally I prefer the #if/#elif.. with #else to catch the error, looks better than a long #if conditional |
| 19:34.37 | rmudgett | Better anyway since each case can actually do work and the final #else can then #error |
| 19:38.50 | coreyfarrell | is there a way to deal with '#define ASTMM_LIBC' ? or do we just let it fail with weird messages? |
| 19:47.02 | *** join/#asterisk-dev tzafrir (~tzafrir@bzq-218-28-58.cablep.bezeqint.net) |
| 19:47.02 | *** mode/#asterisk-dev [+o tzafrir] by ChanServ |
| 19:47.42 | rmudgett | I don't think we should worry about that. ASTMM_LIBC is used improperly there. |
| 19:49.00 | *** join/#asterisk-dev roramirez (~rodrigo@190.54.225.167) |
| 19:51.05 | coreyfarrell | will update the review once I build and use 'nm' to check which symbols are being used with and without malloc_debug. |
| 19:59.20 | roramirez | coreyfarrell: i have a new review the sorting for manager actions |
| 20:00.00 | coreyfarrell | roramirez: great, i'll check it out later (my working copies of master are busy right now) |
| 20:00.39 | roramirez | good!, thanks coreyfarrell |
| 20:09.19 | *** join/#asterisk-dev SaintMoriarty (~SaintMori@ip68-228-236-156.ph.ph.cox.net) |
| 20:53.56 | *** join/#asterisk-dev [TK]D-Fender (~joe@64.235.216.2) |
| 22:16.53 | *** join/#asterisk-dev DivideBy0 (~DivideBy0@unaffiliated/divideby0x0) |
| 22:36.26 | *** join/#asterisk-dev gtjoseph (~gtj0@unaffiliated/gtj) |
| 23:33.55 | *** part/#asterisk-dev kharwell (kharwell@nat/digium/x-axpfdzbqekpyxvyg) |
| 23:37.51 | *** join/#asterisk-dev pchero (~pchero@0x555140b5.adsl.cybercity.dk) |
| 23:51.39 | *** join/#asterisk-dev superscrat (~asanders@173-17-133-2.client.mchsi.com) |