Submitter | Iulian Stana |
---|---|
Date | Sept. 3, 2013, 8:30 p.m. |
Message ID | <1d20e8ae28ff6b4b39eb.1378240244@doppler> |
Download | mbox | patch |
Permalink | /patch/2310/ |
State | Deferred, archived |
Headers | show |
Comments
:::: # HG changeset patch :::: # User Iulian Stana <julian.stana at gmail.com> :::: # Date 1378234875 -10800 :::: # Tue Sep 03 22:01:15 2013 +0300 :::: # Node ID 1d20e8ae28ff6b4b39eb527888c3869ab5e98b86 :::: # Parent 3b4648c6815ba9e1c46a7e2d1184a72952c6e537 :::: hg_close: close the connection with hg cmd server for the given handle :::: :::: The Handle paramater it's pass by address because I want to set the handle :::: pointer to NULL. It's a defence action. In this way I can test if a handle is :::: set or not. :::: :::: diff --git a/client.c b/client.c :::: --- a/client.c :::: +++ b/client.c :::: @@ -149,5 +149,28 @@ :::: return handle; :::: } :::: :::: +/* :::: + * Close the connection for the given handle. :::: + * */ :::: +int hg_close(hg_handle **handle) I rebounce a comment from Laurence Holst (Aug 6th) http://thread.gmane.org/gmane.comp.version-control.mercurial.devel/62348/focus=62498 that I second, and didn't get enough attention. It's about the prototype int hg_close(hg_handle **handle); Kevin: :::: One '*' too many, unless you're planning on closing :::: multiple handles at once... Iulian: :::: It's a defense action, I need to set the pointer to NULL, :::: to be sure that there will be no other calls after hg_close. :::: (We can assume that our users will not make this kind of action, :::: then I will change to only one '*') Laurence: :::: That does not help if the handle is also in another variable :::: (e.g. of a calling function). :::: :::: I do think it’s smart to defend against this, but it’s better prevented :::: by always handing out unique handles that will cause an error :::: if reused after closing. :::: +{ :::: + if(!(*handle)) { :::: + errno = EINVAL; :::: + return -1; :::: + } :::: + :::: + if(kill((*handle)->childpid, SIGKILL) < 0){ :::: + return -1; :::: + } :::: + close((*handle)->p_read); :::: + close((*handle)->p_write); :::: + :::: + free_data(*handle); :::: + free(*handle); :::: + *handle = NULL; :::: + :::: + return 0; :::: +} :::: + :::: #include "client.h" :::: #include "utils.h" :::: diff --git a/client.h b/client.h :::: --- a/client.h :::: +++ b/client.h :::: @@ -71,7 +71,19 @@ :::: * */ :::: hg_handle *hg_open(const char *path, char *encoding); :::: :::: - :::: +/** :::: + * \brief Close the connection for the given handle. :::: + * :::: + * Erase the handle and free the memory :::: + * \param handle The handle of the connection that I want to close :::: + * \retval 0 if successful :::: + * \retval -1 to indicate an error, with errno set appropriately. :::: + * :::: + * errno can be: :::: + * - EINVAL - Invalid argument ( handle it's set to a null pointer) :::: + * - kill(2) command errors :::: + * */ :::: +int hg_close(hg_handle **handle); :::: :::: :::: #endif
Patch
diff --git a/client.c b/client.c --- a/client.c +++ b/client.c @@ -149,5 +149,28 @@ return handle; } +/* + * Close the connection for the given handle. + * */ +int hg_close(hg_handle **handle) +{ + if(!(*handle)) { + errno = EINVAL; + return -1; + } + + if(kill((*handle)->childpid, SIGKILL) < 0){ + return -1; + } + close((*handle)->p_read); + close((*handle)->p_write); + + free_data(*handle); + free(*handle); + *handle = NULL; + + return 0; +} + #include "client.h" #include "utils.h" diff --git a/client.h b/client.h --- a/client.h +++ b/client.h @@ -71,7 +71,19 @@ * */ hg_handle *hg_open(const char *path, char *encoding); - +/** + * \brief Close the connection for the given handle. + * + * Erase the handle and free the memory + * \param handle The handle of the connection that I want to close + * \retval 0 if successful + * \retval -1 to indicate an error, with errno set appropriately. + * + * errno can be: + * - EINVAL - Invalid argument ( handle it's set to a null pointer) + * - kill(2) command errors + * */ +int hg_close(hg_handle **handle); #endif