| 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