| 00:02.50 | coreyfarrell | for the finding about accessing memory stored by weakproxy from destroy_fn, I'm open to suggestions for making that comment clearer. maybe I should add an example to the comment? |
| 00:04.29 | rmudgett | I'm not sure what you are trying to say there. Which destructor is this suppose to be? The weakproxy or the obj's? |
| 00:04.53 | rmudgett | Neither destructor should know about the other. |
| 00:06.20 | coreyfarrell | if realobj->name == weakproxy->name, the destructor for weakproxy would be responsible for freeing name. |
| 00:07.13 | rmudgett | Just saw your review reply. I don't think that is a reasonable thing to do since the lifetimes should not be tied like that. |
| 00:07.53 | rmudgett | They are two separate objects and should take care of their own memory. |
| 00:08.51 | coreyfarrell | it's impossible for the weakproxy to be destroyed before the realobj it points to, and once realobj points to a weakobj it cannot be repointed |
| 00:10.35 | rmudgett | Relying on that relationship means that the realobj can never be created without a proxy. |
| 00:17.23 | coreyfarrell | that's not something that astobj2.c decides, it's up to the code that implements whatever realobj/weakproxy is. if the object always has a weakproxy then it can decide to share memory as long as the weakproxy owns anything that is shared. |
| 00:18.47 | rmudgett | True. The example code you supplied in the review response helps show the idea you need to get across. Put that in the comment. |
| 00:18.53 | coreyfarrell | plus ao2_weakproxy_set_object documents that the weakproxy must be set before the obj is accessible to outside code, so a type of object would either have weakproxy's or not. |
| 00:19.44 | coreyfarrell | you can't create realobj, make it reachable to other threads, then add a weakproxy (without breaking the rules anyways) |
| 00:21.26 | coreyfarrell | thanks for discussing directly, I'll update the review this weekend. have a good one! |
| 00:22.03 | rmudgett | Reminder: mjordan is going to make origsvn read-only Saturday |
| 00:23.01 | coreyfarrell | I'll still be able to post to reviewboard though right? this isn't on a /team/ branch, it's just on reviewboard and a working copy |
| 00:23.22 | rmudgett | Yes. That should still work. |
| 00:24.50 | rmudgett | reviewboard is still going to be around for awhile. Adding new reviews won't be a good idea because the patch still would have to go up to gerrit for review anyway. |
| 00:25.24 | rmudgett | That and the code reviewboard can access is going to get stale. |
| 00:26.27 | coreyfarrell | I'm an svn hater, so I'll be moving the review to gerrit as soon as it's available :) |
| 01:31.52 | *** join/#asterisk-dev superscrat (~asanders@173-17-133-2.client.mchsi.com) |
| 02:40.58 | *** join/#asterisk-dev sgriepentrog (~sgriepent@162-239-32-132.lightspeed.brhmal.sbcglobal.net) |
| 03:11.05 | *** join/#asterisk-dev superscrat (~asanders@173-17-133-2.client.mchsi.com) |
| 04:10.32 | *** join/#asterisk-dev jagruti_ (~root@182.70.116.47) |
| 04:10.37 | *** part/#asterisk-dev jagruti_ (~root@182.70.116.47) |
| 05:45.46 | *** join/#asterisk-dev oej (~oej@h87-96-134-129.dynamic.se.alltele.net) |
| 05:45.46 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 08:12.00 | *** join/#asterisk-dev oej (~oej@h87-96-134-129.dynamic.se.alltele.net) |
| 08:12.00 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 09:25.00 | *** join/#asterisk-dev oej (~oej@h87-96-134-129.dynamic.se.alltele.net) |
| 09:25.00 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 10:33.49 | *** join/#asterisk-dev oej (~oej@h87-96-134-129.dynamic.se.alltele.net) |
| 10:33.50 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 10:41.30 | *** join/#asterisk-dev infobot_ (ibot@rikers.org) |
| 10:41.30 | *** topic/#asterisk-dev is Asterisk Development Discussion -=- http://www.asterisk.org/developers -=- Tier 2 and 3.14159265 support is in #asterisk -=- Asterisk 13.0.0 now released. Do the release dance! -=- Follow on Twitter at @AsteriskDev |
| 11:52.46 | *** join/#asterisk-dev danjenkins (~anonymous@cpc11-folk2-2-0-cust171.1-2.cable.virginm.net) |
| 12:06.34 | *** join/#asterisk-dev danjenkins (~anonymous@cpc11-folk2-2-0-cust171.1-2.cable.virginm.net) |
| 12:31.19 | *** join/#asterisk-dev danjenkins (~anonymous@cpc11-folk2-2-0-cust171.1-2.cable.virginm.net) |
| 14:33.49 | *** join/#asterisk-dev [TK]D-Fender (~joe@64.235.216.2) |
| 15:00.39 | *** join/#asterisk-dev mjordan (mjordan@nat/digium/x-cvkzneqoilbswexr) |
| 15:00.39 | *** mode/#asterisk-dev [+o mjordan] by ChanServ |
| 15:14.42 | *** join/#asterisk-dev sgriepentrog (~sgriepent@162-239-32-132.lightspeed.brhmal.sbcglobal.net) |
| 15:16.08 | *** join/#asterisk-dev oej (~oej@h87-96-134-129.dynamic.se.alltele.net) |
| 15:16.08 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 16:05.11 | *** join/#asterisk-dev gtjoseph (~gtj0@unaffiliated/gtj) |
| 16:33.12 | *** join/#asterisk-dev oej (~oej@h87-96-134-129.dynamic.se.alltele.net) |
| 16:33.12 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 16:44.54 | *** join/#asterisk-dev superscrat (~asanders@173-17-133-2.client.mchsi.com) |
| 17:05.18 | coreyfarrell | gtjoseph: so I noticed on a bamboo email that tests/channels/pjsip/config_wizard/hints is failing.. did we do that? :( |
| 17:05.54 | gtjoseph | well, I did. :) |
| 17:06.10 | gtjoseph | and I just fixed it. there was a missing variable in the config file |
| 17:08.04 | coreyfarrell | well I say we since I assume it's one of the reviews I gave +1. anyways glad to hear it's fixed |
| 17:09.08 | gtjoseph | actually I don't think this test ever worked. |
| 17:09.24 | gtjoseph | not sure how i didn't catch it before |
| 17:10.26 | coreyfarrell | hmm failing since apr 4th.. guess you're right that's before the changes we made |
| 17:10.55 | gtjoseph | yep. that test was dependent on 13.3 so it probably wasn't run before then |
| 17:12.14 | *** join/#asterisk-dev oej (~oej@h87-96-134-129.dynamic.se.alltele.net) |
| 17:12.15 | *** mode/#asterisk-dev [+o oej] by ChanServ |
| 17:12.43 | coreyfarrell | this makes me wish there was a way to: ./runtests.py -M res_pjsip_config_wizard - and have the system run all tests that will exercise that module. would probably need to improve listing of dependencies in some tests |
| 17:53.10 | mjordan | fyi: the asterisk repo is up in gerrit |
| 17:53.14 | mjordan | haven't done any 'fixes' yet |
| 17:53.22 | mjordan | so it's just the bare SVN import |
| 18:00.21 | gtjoseph | mjordan: i've got a .gitignore file if you need it. |
| 18:00.26 | *** join/#asterisk-dev danjenkins (~anonymous@cpc11-folk2-2-0-cust171.1-2.cable.virginm.net) |
| 18:02.29 | mjordan | gtjoseph: sure! |
| 18:02.40 | mjordan | we don't have a .gitreview file either |
| 18:02.46 | gtjoseph | i can do both |
| 18:03.03 | gtjoseph | want me to just push them with a review? |
| 18:03.13 | mjordan | sure |
| 18:03.20 | mjordan | much appreciated; have to go CostCo shopping |
| 18:03.28 | gtjoseph | :) |
| 18:50.11 | *** join/#asterisk-dev danjenkins (~anonymous@cpc11-folk2-2-0-cust171.1-2.cable.virginm.net) |
| 19:32.09 | gtjoseph | coreyfarrell: i tend to use ** in .gitignore to remind me that it will match in any directory. In our case '*' means same thing. |
| 19:32.41 | gtjoseph | if it's going to trip people up, i'll change back to '*' |
| 19:46.41 | coreyfarrell | gtjoseph: up to you (and others who review).. my finding was that you were missing some patterns. for the '**' that was more of a question |
| 19:47.30 | gtjoseph | no worries. they're semantically the same in this case. i'll use '*'. |
| 19:54.52 | *** join/#asterisk-dev danjenkins (~anonymous@cpc11-folk2-2-0-cust171.1-2.cable.virginm.net) |
| 20:07.44 | gtjoseph | coreyfarrell: did you see my review via the dev list or were you looking at gerrit directly? |
| 20:17.36 | coreyfarrell | gtjoseph: i saw the dev list then went to gerrit |
| 20:17.57 | gtjoseph | hmmm. i didn;t see anything to the list, just to me. |
| 20:18.12 | gtjoseph | might just be my filters |
| 20:52.52 | *** join/#asterisk-dev danjenkins (~anonymous@cpc11-folk2-2-0-cust171.1-2.cable.virginm.net) |
| 22:12.00 | *** join/#asterisk-dev sgriepentrog (~sgriepent@2602:30a:2ef2:840:20c:29ff:fe4f:cac8) |
| 22:56.43 | *** join/#asterisk-dev sgriepentrog (~sgriepent@2602:30a:2ef2:840:20c:29ff:fe4f:cac8) |