[stgt] [PATCH] mgmt and concat_buf

[FUJITA Tomonori]

On Tue, 14 Feb 2012 19:10:40 +0200
Alexander Nezhinsky <alexandern at mellanox.com> wrote:

> This patch introduces concat_buf mechanism (based on open_memstream)
> to dynamically produce response buffers for management tasks in tgtd and
> request buffers for management tasks in tgtadm. This eliminates the need
> in special macros like shprintf() and spares explicit checkguards against
> buffer overflow.
> 
> concat_buf api is defined in util.h, as inline functions to avoid
> duplication in tgtadm.c. Should consider separation into a separate file
> in the future.
> 
> concat_printf() is based on FILE* produced by open_memstream(). This mechanism
> takes care of the dynamic allocation and possible re-allocation of memory on
> the as-needed basis. File descriptor is stored within struct concat_buf,
> and is used transparently to write to a file through concat_write().
> 
> Management task buffers data segments are split into request and response
> which makes the code more readable and less error-prone. 
> In tgtd the response buffer is implemented using concat_buf, while in tgtadm
> it's the request buffer that uses concat_buf.
> 
> Thus the changes influence mainly the SHOW commands of tgtd (those producing
> some output). Upon concat_buf finalization the dynamically allociated buffer
> is stored directly to the mtask response descriptor. No need to worry
> about memory re-allocation anymore, it is taken care of by concat_buf
> mechanism. Concatenated buffer descriptor is stored within struct concat_buf,
> and is used transparently to write to a file through concat_write().
> 
> Using struct concat_buf and its associated concat_printf() variant
> of snprintf(), replacing buffer_check() and _buffer_check() macros.
> This spares explicit checkguards against buffer overflow, and by
> explicit passing of concat_buf as the function argument allows
> nested calls to functions which append more lines to the buffer.
> 
> Using concat_buf api highlights a problem pre-existing in the mgmt code:
> a mix of the regular linux error codes (like ENOMEM) and enum tgtadm_errno
> (like TGTADM_NOMEM). Sometimes positive values returned correspond to
> tgtadm_errno, sometimes to the number of bytes written. Thus another
> step to increase reliability is making the usage of "enum tgtadm_errno"
> explicit throughout the code and avoiding mixup with other error types. 
> This also uncovers many small glitches in the error-handling flow
> (some already present in the patch, some are to be added yet).
> 
> enum tgtadm_errno is typedef'ed to tgtadm_err for brevity.
> As C does not provide for strict type checking for enums and mixes them
> freely with integers, some extra effort should be made to separate between
> these two types of error codes.
> 
> These changes also make the code more readable by explicitly defining some
> functions as acting on behalf of the management tasks by making them return
> tgtadm_err.
> 
> Signed-off-by: Alexander Nezhinsky <alexandern at mellanox.com>
> ---
>  usr/bs.c           |    6 +-
>  usr/bs_aio.c       |    4 +-
>  usr/bs_rdwr.c      |    2 +-
>  usr/bs_sg.c        |    8 +-
>  usr/bs_ssc.c       |    2 +-
>  usr/bs_thread.h    |    4 +-
>  usr/driver.c       |    2 +-
>  usr/driver.h       |    6 +-
>  usr/iscsi/conn.c   |    2 +-
>  usr/iscsi/iscsid.h |   32 ++--
>  usr/iscsi/isns.c   |   22 +--
>  usr/iscsi/target.c |  154 ++++++----------
>  usr/mgmt.c         |  498 ++++++++++++++++++++++++++++------------------------
>  usr/mmc.c          |   10 +-
>  usr/osd.c          |    4 +-
>  usr/sbc.c          |    4 +-
>  usr/scc.c          |    4 +-
>  usr/smc.c          |   61 ++++---
>  usr/spc.c          |   43 +++--
>  usr/spc.h          |   12 +-
>  usr/ssc.c          |    9 +-
>  usr/target.c       |  261 +++++++++++++--------------
>  usr/tgtadm.c       |  105 ++++++------
>  usr/tgtadm_error.h |    4 +
>  usr/tgtd.h         |   53 +++---
>  usr/util.h         |   88 ++++++++--
>  26 files changed, 727 insertions(+), 673 deletions(-)

Applied, thanks.

Very sorry for the delay.

--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html