{"action":"created","issue":{"url":"https://api.github.com/repos/krb5/krb5/issues/423","repository_url":"https://api.github.com/repos/krb5/krb5","labels_url":"https://api.github.com/repos/krb5/krb5/issues/423/labels{/name}","comments_url":"https://api.github.com/repos/krb5/krb5/issues/423/comments","events_url":"https://api.github.com/repos/krb5/krb5/issues/423/events","html_url":"https://github.com/krb5/krb5/pull/423","id":139343894,"number":423,"title":"Add send and receive hooks for sendto_kdc","user":{"login":"gladiac","id":545480,"avatar_url":"https://avatars.githubusercontent.com/u/545480?v=3","gravatar_id":"","url":"https://api.github.com/users/gladiac","html_url":"https://github.com/gladiac","followers_url":"https://api.github.com/users/gladiac/followers","following_url":"https://api.github.com/users/gladiac/following{/other_user}","gists_url":"https://api.github.com/users/gladiac/gists{/gist_id}","starred_url":"https://api.github.com/users/gladiac/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gladiac/subscriptions","organizations_url":"https://api.github.com/users/gladiac/orgs","repos_url":"https://api.github.com/users/gladiac/repos","events_url":"https://api.github.com/users/gladiac/events{/privacy}","received_events_url":"https://api.github.com/users/gladiac/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":11,"created_at":"2016-03-08T17:41:15Z","updated_at":"2016-03-14T15:48:20Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/krb5/krb5/pulls/423","html_url":"https://github.com/krb5/krb5/pull/423","diff_url":"https://github.com/krb5/krb5/pull/423.diff","patch_url":"https://github.com/krb5/krb5/pull/423.patch"},"body":"This add hooks which will be called before sending or after receiving a packet from the KDC.\r\n\r\nThis is work in progress, I post here for early comments so we can discuss naming of functions. Also we can decide how much tests would be needed."},"comment":{"url":"https://api.github.com/repos/krb5/krb5/issues/comments/196377167","html_url":"https://github.com/krb5/krb5/pull/423#issuecomment-196377167","issue_url":"https://api.github.com/repos/krb5/krb5/issues/423","id":196377167,"user":{"login":"greghudson","id":1131421,"avatar_url":"https://avatars.githubusercontent.com/u/1131421?v=3","gravatar_id":"","url":"https://api.github.com/users/greghudson","html_url":"https://github.com/greghudson","followers_url":"https://api.github.com/users/greghudson/followers","following_url":"https://api.github.com/users/greghudson/following{/other_user}","gists_url":"https://api.github.com/users/greghudson/gists{/gist_id}","starred_url":"https://api.github.com/users/greghudson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/greghudson/subscriptions","organizations_url":"https://api.github.com/users/greghudson/orgs","repos_url":"https://api.github.com/users/greghudson/repos","events_url":"https://api.github.com/users/greghudson/events{/privacy}","received_events_url":"https://api.github.com/users/greghudson/received_events","type":"User","site_admin":false},"created_at":"2016-03-14T15:48:19Z","updated_at":"2016-03-14T15:48:19Z","body":"For the setter functions, I would go with krb5_set_kdc_send_hook() and krb5_set_kdc_recv_hook().  I would also make them return void, and allow NULL values of func (to disable the relevant hook after setting one).  The hook type names (with \"pre_send\" and \"post_recv\") can remain as they are, I think, but I would name the context fields \"kdc_send_hook\" and \"kdc_recv_hook\".\r\n\r\nFor the send hook type, I would rename \"new_message\" to \"new_message_out\" and \"new_reply\" to \"reply_out\".\r\n\r\nFor the recv hook type, I would rename \"new_reply\" to \"new_reply_out\".  I would also allow the hook to return an error code (for example, if it runs out of memory trying to construct a new reply).\r\n\r\nIf the send hook returns a non-null but empty reply, that should still be used as the reply; checking for non-zero length seems counterproductive.\r\n\r\nYour code for copying hook replies isn't correct; perhaps what I said about using krb5_copy_data() was misunderstood?  I suggest using  \"*reply = *hook_reply; free(hook_reply);\" instead.  Of course you won't want to do this if reply is NULL, so I would just immediately jump to cleanup if the send hook returns nonzero, and then conditionalize on non-null reply.\r\n\r\nFreeing the hook new_message output from the send hook should not be contingent on a recv hook being set; you can do it in the cleanup handler.  Also, if a new_message is given, we have no further use for the input message, so we can just set \"message = new_message;\" rather than conditionalizing every use of message.\r\n\r\nRecurring style issues (don't worry too much about these, I can fix them when pushing): we don't put braces around control statement bodies if they fit on one line.  Comments should usually be complete, punctuated sentences.  We don't fall into the \"one line per argument\" style when calling functions; we always put multiple arguments on one line when they fit.  Calls through function pointers can be written as \"ptr(args);\" rather than \"(*ptr)(args)\" if ptr involves a dereference, so \"retval = context->kdc_send_hook(...)\" and so forth."}}