On Tue, 14 Feb 2012 19:10:40 +0200 Alexander Nezhinsky <alexandern@xxxxxxxxxxxx> 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@xxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html