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

Pierre-Yves David pierre-yves.david at ens-lyon.org
Mon Jul 15 04:10:33 CDT 2013


Here is a simple review of this header:

> diff --git a/client.h b/client.h
> new file mode 100644
> --- /dev/null
> +++ b/client.h
> @@ -0,0 +1,146 @@
> +#ifndef _CLIENT_H_
> +#define _CLIENT_H_
> +#include <stdint.h>
> +#include <errno.h>
> +
> +/*
> + * This structure contains the variables for the header.
> + * The channel will stock a letter that will indicate the type of channel.
> + * The length will stock the size of data that will be send or the maxim data
> + * that can be received by the server.
> + * */
> +typedef struct hg_header{
> +    char channel;
> +    uint32_t length;
> +} hg_header;
> +
> +/*
> + * 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 ?

> + */
> +typedef struct hg_handle{
> +    int wpipe[2];
> +    int rpipe[2];

so wpipe is server stdin, rpipe is server stdout. Were have stderr been?

> +    pid_t childpid;
> +    hg_header header;
> +} hg_handle;
> +
> +
> +/*
> + * 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?

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

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

What's the default?

> + * \return A handle for this connection. 
> + *        NULL to indicate an error, with errno set appropriately.
> + * errno can be :
> + *      ECONNREFUSED
> + *      EFAULT - for a bad path address
> + *      EINVAL - for a bad encoding
> + *      All the errors that can be provide from exec
> + * */
> +hg_handle *hg_open(const char *path, char *encoding);
> +
> +/*
> + * 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?

> + * */
> +int hg_close(hg_handle *handle);
> +
> +/*
> + * Sending a command to the mercurial command server, through the given handle.
> + * \param handle The handle of the connection, wherewith I want to communicate
> + * \param command An array of pointers to null-terminated strings that represent
> + *                the argument list available to the new command.
> + * \return   0 if successful
> + *          -1 to indicate an error, with errno set appropriately.
> + * 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.

> +/*
> + * 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?

> + *                 the data to be read is smaller than nbytes, all data is saved 
> + *                 in the buffer.
> + * \return   The number of bytes that were read
> + *          -1 to indicate an error, with errno set appropriately.
> + * errno can be:
> + *      EINVAL
> + *      EIO
> + *      ECOMM
> + * */
> +int hg_rawread(hg_handle *handle, char *buffer, size_t sizebuff);
> +
> +/*
> + * Will write the buffer to server for the connection establish by the handle.
> + * This function will be used when one of the input channels will be received 
> + * from the command server. ('I' or 'L' channels)
> + * \param handle The handle of the connection, wherewith I want to communicate
> + * \param buffer A null terminated character string of the content to write.

I'm sure there can be input with null character (bundle?).

> + * \param sizebuff The number of bytes to write 
> + * \return   The number of bytes that were written
> + *          -1 to indicate an error, with errno set appropriately.
> + * errno can be:
> + *      EINVAL
> + *      EIO
> + *      ECOMM
> + * */
> +int hg_rawwrite(hg_handle *handle, const char *buffer, size_t sizebuff);
> +
> +/*
> + * Before you read or write data, you will want to know for what kind of data is 
> + * server waiting, an input data or an output data.
> + * This function will return the current channel for the connection established
> + * by the handle.
> + * \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.

> +/*
> + * 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 ?

> +/*
> + * The server tell use that he finished his action when the 'r' channel is send.
> + * This function will get the exitcode from the server, in as a measure to tell
> + * that the command was finished.
> + * \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
> + * */
> +int hg_exitcode(hg_handle *handle);

-- 
Pierre-Yves



More information about the Mercurial-devel mailing list