[PATCH -V2 RFC] Header file for hglib - Level 0 API

Iulian Stana julian.stana at gmail.com
Mon Jul 15 13:07:18 CDT 2013


> > + * This structure will be use the handle the connection with the server.
> > + * He contains 2 pipes for the biderectional connection.
> > + * The childpid, the pid of the process that will open the connection
> > + * The header that will contain the current action-header.
>
> Action-header, what is an action-header ?
>

The header for the current command (channel + data length).


> > + */
> > +typedef struct hg_handle{
> > +    int wpipe[2];
> > +    int rpipe[2];
>
> so wpipe is server stdin, rpipe is server stdout. Were have stderr been?


I need the pipes for the bidirectional communication. You don't need some
dedicate pipe for stdin/stdout/stderr, the user receive from server a
specific
channel ('o' - output, 'e' - error, 'I' - input, etc). According to those
channels
you know from what stream your data comes.


> > + * Open the connection with the mecudial command server.
> > + * The handle structure will be allocated.
> > + * \param path The path to the repository wherewith I want to create a
> connection
> > + *        NULL argument means the repository in which I am
>
> 1. "The repository in which I am", do you mean the first repository found
> in PWD hierarchy?
>

Yes.


> 2. You need to support handle without repo for command without repo.


I don't really know what do you mean ... You cannot open a connection with
nothing, you
need a repo.
In python-hglib there are some separate functions for clone/init that first
create the
repo and than establish the connection. [1]
And for those commands that are not require a connection, there is no need
for a
handle and those commands will be separate.

[1]:
http://selenic.com/repo/python-hglib/file/c13b99b01008/hglib/__init__.py


> > + * \param encoding Will set HGENCODING to the given encoding
> > + *        NULL argument means the default encoding
>
> What's the default?
>

The default encoding is UTF-8, I will specify.

> +/*
> > + * 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
> > + * \return   0 if successful
> > + *          -1 to indicate an error, with errno set appropriately.
> > + * errno can be:
> > + *      EINVAL
>
> In what condition?
>

Invalid argument. Handle is a null pointer.

> + * errno can be:
> > + *      EINVAL
> > + *      EIO
> > + *      ECOMM
> > + * */
> > +int hg_rawcommand(hg_handle *handle, char *const command[]);
>
> See Matt comment about space in argument. But I believe you are currently
> reworking this aspect already.


I already solved this issue.


>

> +/*
> > + * Reading some unparse data from the server. Will read just a 'line',
> the
> > + * header that is received from server and the data that comes after
> the header
> > + * \param handle The handle of the connection, wherewith I want to
> communicate
> > + * \param buffer A character array where the read content will be
> stored.
> > + * \param sizebuff The number of bytes to read before truncating the
> data. If
>
> Truncating the data‽ why would you ever truncate the data?


My bad, I will change this. More likely you read just sizebuff from the
server.

> + * \param handle The handle of the connection, wherewith I want to
> communicate
> > + * \return   0 if successful
> > + *          -1 to indicate an error, with errno set appropriately.
> > + * errno can be:
> > + *      EINVAL
> > + *      EIO
> > + *      ECOMM
> > + * */
> > +char hg_channel(hg_handle *handle);
>
> Not sure to see what this function is for.
>

You need to know what's your next step. If you are still reading from cmd
server or
if you need to write data, or if you are already on the end of the current
command.


>
> > +/*
> > + * Sometimes, the user needs the entire header. This is a way to do
> that.
> > + * \param handle The handle of the connection, wherewith I want to
> communicate
> > + * \return  The hader stucture for the given handle.
> > + *          NULL to indicate an error, with errno set appropriately.
> > + * errno can be:
> > + *      EINVAL
> > + * */
> > +hg_header hg_channel(hg_handle *handle);
>
> same name than above ?


My bad, I already change this.


-- 
O zi buna,
Iulian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20130715/4948b6d8/attachment.html>


More information about the Mercurial-devel mailing list