[PATCH 05 of 14 RFC c-hglib:level0] hg_close: close the connection with hg cmd server for the given handle

Giovanni Gherdovich g.gherdovich at gmail.com
Wed Sep 11 23:49:23 CDT 2013


:::: # 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


More information about the Mercurial-devel mailing list