/srv/irclogs.ubuntu.com/2009/01/28/#upstart.txt

* Keybuk finally gets around to proving some of the bugs sadmac fixed in 0.502:12
sadmacKeybuk: which ones?02:49
Keybuk$ENV's replacement being the same length as the variable name+$ for example02:52
sadmacoh yes02:53
KeybukI'm going to be strict from now on ;) all bug fixes must come with test cases02:53
sadmacnot a problem02:54
* sadmac goes back to writing tests02:54
sadmacKeybuk: test_com.netsplit.Nih.Test_object: tests/com.netsplit.Nih.Test_impl.c:766: my_setup: Assertion `server != ((void *)0)' failed.02:58
sadmacKeybuk: I started getting these failures writing my testcases, but reverting my changes doesn't fix them02:58
Keybukerr02:58
Keybukthat implies your test d-bus server isn't running?02:58
Keybukhave you checked it got killed ok before?02:58
Keybukusually it's test_dbus-daemon02:58
sadmacI see a test_com.netspli02:59
sadmacthat's probably it02:59
sadmacahh, segmentation fault03:00
sadmacsomething strangely comforting about knowing its your own fault after one of those phantom errors.03:00
sadmacKeybuk: how much later will you be around?03:05
Keybuknot much03:06
Keybukwhy?03:06
sadmacKeybuk: I have the async api and one test case. I want to show it to you before I write more so I know if the API as at least right. But I have a segfault left to scrub out so its not quite ready :)03:07
Keybukcool03:11
KeybukI may be around long enough ;)03:11
sadmacKeybuk: dbus_shutdown is triggering the segfault. Its being called from the teardown function (which I haven't touched). Only thing I can think of that would do it is a call to dbus_connection_read_write_dispatch (conn, -1);03:12
sadmacseems like without that it didn't segfault (though the test didn't really run)03:12
Keybukthat implies you've got your references mixed up somewhere03:23
sadmacah03:24
sadmacwhat's happening then?03:24
* sadmac sees definite double-deref03:25
sadmacdunno how that could do this, but its a bug03:26
sadmacKeybuk: \o/03:26
sadmacKeybuk: how do you want this then?03:27
Keybukgot a bzr tree?03:35
sadmacKeybuk: yeah, writing the commit notes now...03:37
sadmacI had more outstanding than I thought.03:37
sadmacKeybuk: https://code.launchpad.net/~cjdahlin/libnih/async03:46
Keybukok, give me a few minutes to finish up this bit03:50
Keybukand I'll take a look03:50
sadmack03:52
Keybukurgh04:08
Keybukthe valgrind attacks.  you die04:08
Keybuk(not your code, mine)04:09
Keybukhttp://rafb.net/p/EzdMTh82.html04:09
Keybuknothing worse than an invalid write into a freed block where the write and free are inside the same function04:10
Keybuk(and that function is "free" :p)04:10
sadmacot04:10
sadmac*oy04:10
sadmacisn't it 4am over there?04:16
Keybukyes04:22
Keybukdon't you ever stay up all night hacking? :)04:22
Keybukwhy names?04:25
Keybukunless I'm missing something, the dispatch function doesn't take the method call arguments04:28
KeybukI'd generate a typedef for the callback function and put it in the .h file04:31
Keybukand use that04:31
Keybukif callback is NULL, should send without expected reply04:31
KeybukNihAsyncNotifyData should be in the .c file, since it should never be exposed04:32
Keybukshould be typedef'd to that name04:32
Keybukmember names should line up04:32
Keybukuserdata -> data04:33
Keybukproxy should probably go first too04:33
sadmacKeybuk: for which callback?04:33
Keybukargument to the async dispatch function04:34
sadmacKeybuk: ah, nm, I misunderstood for a moment04:34
Keybukstruct nih_blah_blah -> NihBlahBlah04:34
KeybukI wouldn't call it that though04:34
Keybukmake it similar to the DBus Name04:34
KeybukNihDBusPendingCall or something04:34
sadmacKeybuk: you omit the CamelCase form on another struct in dbus.h04:34
Keybukdo I?04:35
sadmacpretty sure04:35
KeybukI bet I typedef'd it first ;)04:35
KeybukNihDBusInterface?04:35
Keybukline 44: typedef struct nih_dbus_interface NihDBusInterface;04:35
sadmacKeybuk: ah. so it is.04:36
KeybukI can't quite work out what you're doing with the callback definition in asyncDispatchPrototype() - a typdef would definitely help here04:36
Keybukand then it'd be obvious you've forgotten all about in_args ;)04:37
sadmacKeybuk: line 128304:37
Keybukoh04:37
Keybukthey should go first04:37
sadmacKeybuk: I figured put the consistent-between-all stuff first04:38
Keybukproxy, input arguments, callback, data04:38
Keybukmy_str_to_int (proxy, "foo", get_int, NULL);04:38
Keybukreads the right way then04:38
sadmacyeah, I can see that..04:39
Keybukyou don't actually check whether the pending call completed?04:40
Keybukin fact04:40
Keybukdbus_pending_call_steal_reply doesn't return NULL to indicate out of memory04:40
Keybukit returns NULL to indicate no reply received yet04:41
Keybukin case of timeout, dbus_pending_call_steal_reply returns an error04:41
Keybukin fact, how do we deal with errors at all? :)04:42
sadmacoh yeah, I hadn't done that yet had I?04:42
Keybuk        # FIXME: Reply isn't the best parent for this, but proxy isn't here. 04:42
Keybukcomment doesn't make sense given you use proxy ;)04:42
sadmacoh, fixed it04:42
Keybukshould the proxy be passed to the callback function?04:43
KeybukI think it makes sense for it to be04:43
Keybukin case the callback wants to make more calls04:43
Keybukan error handler is going to look like  void (*error_handler) (void *data, NihDBusProxy *proxy;04:43
Keybukwhich I guess means you have to provide both04:44
Keybukmy_str_to_int (proxy, "foo", get_int, deal_with_error, NULL);04:44
Keybukbut it's probably ok for the error handler to be NULL04:44
Keybukactually04:44
Keybukno it's not, let's make that required if a callback is givne04:45
Keybuknih_alloc (..., struct) -> nih_new ()!04:45
Keybukyou don't provide a free function for the pending call04:46
Keybukthat's kinda important, since otherwise you'll leak the async data structures04:46
Keybukthe free call might want to do something like call the error handler04:46
Keybuksetting some useful error04:46
sadmacok04:47
Keybukso for str_to_int, that'd look something like04:47
Keybuk  my_str_to_int (proxy, "1234", int_reply, error_handler, data);04:47
Keybukwith the others being04:47
Keybukvoid int_reply (void *data, NihDBusProxy *proxy, int num);04:48
Keybukvoid error_handler (void *data, NihDBusProxy *proxy);04:48
Keybuk( the error would be retrieved with nih_error_get() )04:48
sadmacKeybuk: we could save a function pointer by just adding an "int succeeded" to the callback04:55
Keybukerror_handler would be more consistent with other functions04:57
Keybuke.g. nih_io04:57
sadmacKeybuk: did you look at the testcase?04:59
sadmacthere's a bit of conventional intrigue there too04:59
KeybukI did, my eyes have not recovered ;)04:59
sadmacKeybuk: I do like gcc's mandate of the auto keyword in that context05:00
sadmac"hey, we need to break a grammar ambiguity. Know any keywords that aren't busy?"05:00
Keybukhah05:06
sadmacKeybuk: if you have a better way to structure those I'm open to it. What I like about this method is that the tests still read in the order they're run.05:07
KeybukI'll either hate it ...05:07
Keybukor convert everything else to use them ;)05:07
Keybukngggarrrrgh05:08
sadmachuh?05:08
Keybukthe valgrind error can be replicated with the absolute simplest of code05:08
Keybukc = job_class_new (NULL, "foo");05:08
Keybukj = job_new (c, NULL);05:09
Keybuknih_free (c)05:09
sadmacthat's ugly05:09
Keybukthe strange thing is that this should all just work now05:09
Keybukthe whole point of refs is that if you have a pointer, you have a ref05:10
Keybukso they should collapse cleanly *sigh*05:10
sadmacone thing I wish we could do (and I'm nearly certain we can't) is have a NIH_FRAME macro instead of nih_local, and you simply parented things to it to make them have a reference held by the current stack frame05:11
KeybukI considered that approach05:11
KeybukI decided nih_local was nicer05:11
sadmacI like having the frame parent, but the things you'd have to do to make it work would offset any nicety.05:12
sadmacyou'd have to hide a var somewhere in the function somehow to run your cleanup05:12
sadmacor pillage the stack manually05:13
Keybukhaving the frame parent feels a bit like assembler05:13
sadmacor keep a costack and call everything that uses it through wrappers05:13
Keybukjust saying "this is local" feels a bit like python ;)05:13
sadmacI think its the word "frame"05:13
Keybukthe aim is to simplify the code05:13
Keybukyou'd have to do something like:05:14
Keybuk  {05:14
Keybuk    NIH_LOCAL local;05:14
Keybuk    char *str;05:14
Keybuk...05:14
Keybuk     str = nih_strdup (local, "foo");05:14
Keybuk  }05:14
Keybukit kinda gets lost in the fog a bit05:14
sadmacthe need to declare it is what kills it05:14
sadmacand that's what I can't factor out05:14
Keybukyeah05:15
Keybukif that were05:15
Keybuk  {05:15
Keybuk    char *str;05:15
Keybuk...05:15
Keybuk    str = nih_strdup (NIH_LOCAL, "foo");05:15
Keybukthat would be ok05:15
Keybukbut I don't think you can declare variables right there ;)05:15
Keybukand even if you could, the second use of NIH_LOCAL would break it05:15
sadmacwhat I /really/ wish is that you could do this:05:15
Keybuk(and I see what causes this valgrind bug now ...05:15
Keybuk it's the ancient thing-is-in-a-hash-table bug)05:16
sadmacvoid foo (char nih_local *bar) {...05:16
Keybukclass->jobs is a hash table05:16
Keybukjob is in that05:16
sadmacand it would make bar nih_local and /ref/ it05:16
Keybukwhen you free job, it removes itself from the hash05:16
Keybukbut if you free class, the hash should be freed first05:16
Keybuksadmac: huh?05:16
Keybukhow does that work05:17
Keybukit's an arg passed in, it's inherently already ref'd by something and you don't need to care05:17
sadmacKeybuk: pthread_create()05:17
* Keybuk wonders why this valgrind bug doesn't affect old nih_alloc05:17
Keybuktimestamp: Thu 2006-12-21 00:24:05 +000005:18
Keybukmessage:05:18
Keybuk  * nih/io.c (nih_io_message_new): Do not assign a default list05:18
Keybuk  destructor, it's not in a hidden list and there are bad consequences05:18
Keybuk  of freeing a child with a default destructor if the list its in gets05:18
Keybuk  freed first; new rule - default destructors only if the list is hidden05:18
Keybuk  and never freed!05:18
Keybuk...05:18
Keybukespecially since I apparently hit it over 2 years ago :p05:18
sadmacKeybuk: if we go thread-safe, stack references are more important, because a piece of code referencing a function doesn't tell us as much about other pieces of code that might be dealing with the memory area05:19
sadmacthat's when things like this become interesting05:19
sadmacbut back to your valgrind issue... have a fix?05:19
Keybukno, not yet05:26
KeybukI'm trying to understand why it's breaking now first05:26
sadmacKeybuk: while we're here, I had a hypothesis: why do you put the function types on the line above the name when defining and on the same line when declaring?05:28
Keybukcommon C practice05:29
Keybukgrep ^function_your_looking_for *.c05:29
Keybukas a way of locating the code05:29
sadmacpart A confirmed05:30
sadmacKeybuk: part B: do you use cscope?05:30
Keybuknope05:31
sadmacmhm05:31
Keybukwhy?05:31
sadmacKeybuk: the theory is that if you used cscope you wouldn't put the function definition on two lines'05:32
sadmacKeybuk: because you'd be able to find the function no matter how it was formatted05:32
KeybukI use ctags, which are pretty much the same thing05:33
Keybukbut it's something I find useful in other people's code05:33
Keybukso I do it too05:33
Keybukand it has the advantage of making a function definition really obvious05:33
sadmacgives you more space for arguments too05:35
* sadmac used to pride himself on the distant right margin of his VB code05:35
sadmacah was I ever that young?05:35
Keybukheh05:37
Keybukmy C coding style is kinda wacky really05:37
Keybuklike the way I line up argument and variable names05:37
Keybukand if (! foo)05:38
sadmacKeybuk: the putting spaces between function names and open parens kills me. especially now that I'm paid to be in kernel code05:39
Keybukhaha05:39
Keybuksomeone once said that there are only two types of programmers05:39
Keybukthose that put a space between function names and their parameters, and those that don;t05:40
sadmacKeybuk: and the guys who wrote procmail05:40
sadmacthat's definitely a new species05:40
sadmacfd.o's C style guidelines are pretty backward too05:41
sadmac2-space tabs? doing it wrong.05:41
Keybukhere's a question for you then05:41
Keybukif you don't like foo(...)05:42
Keybukdo you do for(...) ?05:42
sadmacKeybuk: no, because for is yellow in vim05:42
sadmacyellow things get a space before the args05:42
sadmacexcept for sizeof05:43
sadmacbecause linus says so05:43
Keybuksizeof shouldn't have ()s05:45
Keybuksizeof int05:45
sadmacyou take the size of nicer things than I do05:46
sadmacsizeof(typeof(**x))05:46
Keybuktypeof doesn't have ()s either05:46
Keybukbesideswhich05:47
Keybuksizeof/typeof is meaningless05:47
Keybuksizeof **x05:47
Keybukis the same05:47
sadmac...whoa...05:47
sadmacyou're right05:47
sadmacKeybuk: what about those people that name the function arguments outside of the declaration header?05:49
Keybukthose people need to stop denying the 80s happened05:50
sadmacor go code COBOL in a hole somewhere05:50
sadmacHOLBOL?05:51
Keybukarguments outside the header is old-style K&R C05:51
Keybukwe program in ANSI C nowadays05:51
sadmacsort of05:51
sadmacKeybuk: can we add cscope.out to the ignorefile?05:55
Keybukno ;)05:57
Keybukfor the same reason I don't put TAGS in there05:57
Keybuk(you should only ignore output expected to be generated by the ordinary build process)05:58
sadmacKeybuk: does bzr have a local ignore?05:59
KeybukI don't think so06:01
sadmacdamn06:01
* sadmac misses git06:01
Keybukoh, yes06:02
Keybuk~/.bazaar/ignore06:02
sadmaccool06:04
sadmacbug!06:04
sadmacas soon as I added a line to ~/.bazaar/ignore, all the object files showed up in bzr status06:05
Keybukoh, I think that's deliberate06:05
Keybukbzr has a built-in global ignore06:05
Keybukif you make a new one, it overrides bzr06:05
sadmaccan I source the normal one?06:06
Keybukhmm06:06
Keybukdid you not have a .bazaar/ignore already ?06:06
Keybukyou just did echo > .bazaar/ignore06:06
Keybukdidn't you?06:06
sadmacFUUUUUUUUUUUUUUUUUUUUUUUUUUUU06:06
Keybukrm it06:06
Keybukrun bzr ignored06:07
Keybukthen edit it ;)06:07
sadmacsuperior usability!06:07
Keybukyou can't use that word06:08
Keybukyou like git06:08
sadmacI /can/ however use my vcs06:09
* sadmac uses interactive rebase to do obscure things you can't even imagine in O(1)06:09
KeybukI can use git too, I just strongly dislike it06:09
KeybukLinus tells you not to rebase06:09
Keybukhe says every time someone rebases, he kills a kitten06:09
sadmacinteractive rebase is better than rebase06:09
sadmacit basically lets you put everything in the repo in any order you want.06:10
sadmacits like using ed on the object files, but you can play louder techno music06:10
Keybukyou're not supposed to fuck with the repo06:11
sadmacI don't get much other action, and the repo understands my needs06:11
sadmacdon't judge me, what we do is beautiful06:11
Keybuk:D06:13
sadmacoh /repo/. I thought you said horses.06:13
Keybukhow equus of you06:13
sadmacI don't think he actually fucked the horse.06:14
sadmacHe stabbed the horse. He rode the horse naked.06:14
sadmacNot sure you could stage a fucking in the way that play was supposed to be staged06:14
Keybukno idea06:18
sadmacits kind of a funny image actually...06:18
sadmachttp://www.asiaarts.ucla.edu/media/images/EQUUS%201.jpg06:18
sadmacthat's pretty funny already. If that broke down into some hardcore I'd be seriously amused.06:19
Keybukheh06:20
* Keybuk spots the basic difference between old nih_alloc and new nih_alloc06:20
sadmaco yeah?06:21
Keybukrevno: 45506:21
Keybukcommitter: Scott James Remnant <scott@netsplit.com>06:21
Keybukbranch nick: libnih06:21
Keybuktimestamp: Mon 2007-10-15 23:14:14 +010006:21
Keybukmessage:06:21
Keybuk  * nih/alloc.c (nih_alloc_using, nih_alloc_reparent, nih_realloc):06:21
Keybuk  Change the order in which children allocations are stored in the06:21
Keybuk  list such that the last allocation is freed first rather than06:21
Keybuk  the other way around.  This solves issues of children being stored06:21
Keybuk  inside an allocated hash table which will be freed first.06:21
Keybukin other words, old nih_alloc frees things in the opposite order to that they are allocated in06:23
sadmacthat working implies that the parenting ordering is a subset of the allocation ordering06:23
sadmacits perfectly possible to:06:24
sadmac1) allocate item with no parent06:24
sadmac2) allocate hash table06:24
sadmac3) stick item in hash table06:24
sadmacin which case this would break06:24
Keybukyeah I never really fixed the original bug06:24
KeybukI just kept moving it around every time I noticed it ;)06:24
sadmacwhat you should do is not free the memory right away but put it into some kind of list, and then free everything in the list only when its know that all the references are stable.06:25
sadmacnih_free (void *item)06:27
sadmac{06:27
sadmac        static int rec_depth = 0;06:28
sadmac        ...06:28
sadmac        rec_depth++;06:28
sadmac        ...06:28
sadmac        nih_insert_item_into_free_list(item);06:28
sadmac        if (! --rec_depth) {06:29
sadmac                nih_free_all_free_list();06:29
Keybukyeah, that's the plan06:29
Keybukfinalise the object first06:29
sadmacKeybuk: where do I move the NihAsyncNotifyData struct so it shows up in the C files?06:37
Keybukyou might have to define something to do that06:38
sadmacmmh06:39
* sadmac thinks about rewriting all this with a templating engine again06:39
Keybuk...with child in older sibling list06:41
KeybukBAD: block 0xacd108 (child->entry.next) freed unexpectedly06:41
Keybukat tests/test_alloc.c:243 (child_destructor_test).06:41
Keybuksweet06:41
sadmachave a testcase then?06:41
KeybukI think so06:42
Keybukthough one passes but fails valgrind06:42
Keybukoh, hah06:42
Keybukbecause I'm an idiot06:42
Keybukmy test case uses TEST_FREE_TAG ... which uses nih_alloc06:42
Keybukcan't test nih_alloc with TEST_FREE_TAG you dolt06:42
Keybukright06:56
KeybukI now have two test cases06:57
Keybukthat both work06:57
Keybukif you use _add () the first test case fails06:57
Keybukif I change it to _add_after () the second test case fails06:57
sadmacKeybuk: ok, last I saw I asked about typedefs07:01
KeybukI didn't see that07:06
sadmac<sadmac> Keybuk: is there a place to put the typedefs for the callbacks07:06
Keybukanything above that?07:07
sadmac01:43  * sadmac fixes a longstanding problem with his vimrc07:07
sadmacthen before that you said you were an idiot07:07
Keybukheh07:08
Keybuknot sure if there's a place in the python code07:08
KeybukI certainly meant to have one, but may not have added that func07:08
Keybukshould go at the top, before externPrototypes07:09
sadmacI'm sleeping now.07:38
Keybukheh07:47
KeybukI'm just about to do that07:47
KeybukI'm worried07:47
Keybukmy code worked first time07:47
KeybukUpstart's test suite largely works too07:54
Keybukthere's a few failures, but I don't _think_ they are related07:54
Keybuk(and frankly, the d-bus stuff works - and that really abuses nih_alloc :p)08:07
Keybukhttp://bazaar.launchpad.net/%7Escott/libnih/trunk/revision/63908:08
Keybukin summary: first call all the destructors, then free them all08:09
Keybukbut rather more efficient without the need to recurse ;)08:09
Keybukright21:23
Keybukyou and me, valgrind...21:23
sadmacKeybuk: what about you, him, and valgrind?21:25
Keybukgoing to fix these remaining test case failures21:26
KeybukI'm also trying to decide whether it's a bug to call nih_free () on an object with a parent21:26
Keybukion_: what do you think?21:26
ion_I’d lean on only allowing to free by unrefing.21:27
ion_uh, towards21:27
Keybukwhat if you never allocated it with a reference in the first place? :)21:27
sadmacKeybuk: I'd lean toward /that/ being a bug21:28
sadmacor rather impossible21:28
Keybukwell, you have to start somewhere?21:28
ion_Ah, right, parentless objects don’t have a reference.21:28
ion_Hmm21:28
Keybukotherwise you'd need a global reference, which is basically the same as NULL ;)21:28
sadmacKeybuk: exactly21:28
Keybukso why not use NULL?21:28
Keybukthe disadvantage of a global reference, and the reason nih_alloc doesn't have one, is that valgrind thinks the object is in use ;)21:29
sadmacKeybuk: yes. Use NULL by all means.21:29
Keybukso doesn't bitch if you forget to free21:29
Keybukthat was the other main reason I opted for "nih_local" and NULL rather than NIH_LOCAL as the parent21:29
sadmacKeybuk: you could have a debug mode where you htonl() the refs that are from the root object21:30
Keybukhuh?21:32
sadmacKeybuk: "encrypt" references from the root object when -DDEBUG so valgrind can't read them21:32
ion_How about just making unref() free parentless objects, and behave as before with objects with parents?21:33
Keybukthat involves having a debug mode though ;)21:35
Keybukion_: nih_unref (ptr, NULL) type thing?21:35
ion_Or maybe make nih_free barf if the object has parents?21:38
Keybukor make nih_free only do something if the object has parents?21:40
ion_How should it be used then?21:46
ion_If you nih_new() without a parent and nih_free() it, nothing would happen, right?21:46
KeybukI mean if the object hasn't parents21:47
Keybukso nih_free() matches nih_new(NULL, ...)21:47
sadmacKeybuk: what's the difference between what you get from nih_alloc(NULL, ...) and what you have in nih_unref() when you've removed the last ref and its about to be freed21:47
Keybukand nih_unref() matches nih_ref() or nih_new(ptr, ...)21:47
Keybuksadmac: none21:47
Keybukif you like, nih_alloc (NULL, ...) gives you something that's unreferenced, but not free21:47
Keybuka bit like gobject's sunken ref ;)21:47
sadmacKeybuk: that's my issue. its identical to the dead state21:47
Keybukit means you can allocate something, add a reference, and then not worry about it21:48
Keybukyou don't have to drop your own21:48
sadmacKeybuk: IMHO such an object persisting for any amount of time is as bad as a pointer to already freed memory persisting for any amount of time.21:49
Keybukit varies21:51
Keybuksome reference-counting systems are strict21:51
Keybukwhen you allocate, what you have returned has a single reference21:51
Keybukif you pass that to anything and it takes a reference, it must add *another* reference21:52
Keybukand if you don't want it anymore, you *must* drop the single reference you had when it was allocated21:52
KeybukD-Bus's is like that21:52
sadmacKeybuk: why does nih_unref() free the object then?21:52
Keybukthe problem with these is that it's easy to forget to drop the reference you got when you allocated it, or easy to forget to take one when you want it21:52
sadmacKeybuk: its a valid object still21:52
Keybukso others have the concept of a floating reference21:52
Keybukwhen you allocate, what you have has only a floating reference21:53
Keybukif anything references it, the floating reference is sunk and turned into a real reference21:53
Keybukso when you pass it to things that take a reference, it gets turned into a real one21:53
Keybukand since that happens, you don't need to worry about unrefing when you don't want it anymore21:53
Keybuk...21:53
Keybuknon-deliberately, nih_alloc turns out to be a bit like the last one21:53
Keybukyou can allocate with a reference, or without21:54
Keybukif you allocate without, it kinda behaves like a non-explicit floating reference21:54
Keybukin reality, it's an unreferenced objetc21:54
Keybukthis works nicely for things like nih_local21:54
Keybuksince that only discards unreferenced objects21:54
Keybukindeed, there's a call for - nih_discard() - which is a safety "I'm done with an unreferenced object, but am not sure that anyone else took a reference" call21:55
Keybukand you can return an object to unrefernced as well - nih_unref_only ()21:55
Keybukthis is all a bit ... strange really ;)21:56
Keybukit's what happens when you design an API by evolution21:56
sadmacKeybuk: I can see the argument for it existing, but I don't like it as a long-term condition21:56
Keybukespecially when you turn a simple halloc rip-off, into a talloc rip-off into something that afaik is unique21:56
sadmacKeybuk: I'd like to think of it as mid-transaction database inconsistency21:56
sadmacKeybuk: its an error for such a thing to exist, but we defer the error on the grounds of "it'll not be that way anymore in a few lines"21:57
ion_Perhaps rename nih_free as nih_discard (deleting the present nih_discard) to avoid confusion and make it free the object if it has no parents.21:57
Keybukion_: err, that would be exactly what nih_discard does right now, no? :p21:57
ion_Indeed.21:57
Keybukthough would it error if it has parents?21:58
sadmacKeybuk: so rather rename nih_discard to nih_free, and kill the current nih_free21:58
Keybuk(nih_discard doesn't)21:58
ion_No, ignore it if it has parents.21:58
ion_It shouldn’t be called nih_free if it might not free the object IMO.21:58
Keybuknih_free always frees right now21:59
Keybukeven if it has parents21:59
KeybukI tend to agree there though, nih_free would be a bit odd if it didn't always free21:59
ion_Yes, i suggest getting rid of it. :-)21:59
sadmacKeybuk: nih_free not always freeing is weird21:59
sadmacKeybuk: but if you have an interest in API consistency (and it /is/ kind of convenient that we don't have to rewrite all that much right now) it might be better to keep the old symbol/22:00
ion_So the norm would be to nih_discard anything you allocated without a parent, and nih_unref anything you referenced. No nih_free.22:00
Keybukheh22:00
KeybukI just made nih_free assert22:01
Keybuk(if there were parents left)22:01
Keybuklots of things broke22:01
ion_:-)22:01
Keybukmostly test cases, by the looks of it22:01
sadmacKeybuk: what I would prefer is that if you are going to allocate without a parent, you /must/ assign the pointer to an nih_local. dunno what C gives us to enforce that though.22:01
Keybukdunno, dig through the gcc ref manual ;)22:03
sadmacI've learned to like that thing22:03
sadmacKeybuk: I wonder what happens if you do void * nih_local my_func () {...22:04
Keybukthe attribute will apply to the function instead of the return value22:05
sadmacKeybuk: but it can't because its not a function attribute22:05
Keybukso gcc will error22:06
Keybukreturn types don't have variable attributes ;)22:06
sadmacprobably22:06
Keybukthe __attribute__ binds right22:06

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!