On Wed, 22 Sep 2010 21:26:13 -0400 Jeff Garzik <jeff@xxxxxxxxxx> wrote: > > So, we go a longer route and re-hook the list of completions > > to a per-server global instead of a client. The patch is straight- > > forward. The only thing we need to be careful is to make sure > > that no outstanding completions are left in the queue before > > freeing a client struct. This is ensured by force-running completions. > Looking at this change again, I don't see how this avoids > use-after-free. If completions exist after state change function leads > one to cli_evt_dispose() -> cli_free(), then cli_write_run_compl() still > calls cli_write_free() with the stale 'cli' pointer. We run completions before freeing in all cases. My patch was correct. > It seems like the real fix is to have the functions in the FSM loop > return an additional piece of information, indicating that 'cli' is no > longer valid. That's kinda backwards. Might as well add refcounts. > client_write's object lifetime should always be a subset of client's > object lifetime. Sure. But that's an argument for refcounting struct cli. > > Speaking of backpointers, I think it would be much cleaner > > to get rid of two-argument format for callback. It stinks of > > special-casing. Either throw a back pointer into the first word > > of buf, or create some kind of object/struct passed as > > argument to atcp_writeq(), that's what I would do. > > It is a common idiom even in GLib that callbacks receive two anonymous > pointers; witness the data type GFunc's 'data' and 'user_data' > arguments: > http://library.gnome.org/devel/glib/stable/glib-Doubly-Linked-Lists.html#GFunc There's a lot of retarged garbage in Glib, just look at their lists. If someone smarter wrote Glib, we would not need struct list_head. Heck in fact queue.h existed before Glib and they still blew it. Oh god and their hash is no better. > This is because typically one has two objects -- a parent and a child -- > with different object lifetime. [] I think your excuses are getting more sophisticated than the code is worth. If you don't see how 2 arguments are special-casing, fine. Just say you like it. -- Pete -- To unsubscribe from this list: send the line "unsubscribe hail-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
