dm-devel.redhat.com archive mirror
 help / color / mirror / Atom feed
From: mwilck@suse.com
To: Christophe Varoqui <christophe.varoqui@opensvc.com>,
	Benjamin Marzinski <bmarzins@redhat.com>
Cc: dm-devel@redhat.com, Martin Wilck <mwilck@suse.com>
Subject: [dm-devel] [PATCH 0/9] multipath-tools: use variable-size string buffers
Date: Thu, 15 Jul 2021 12:52:14 +0200	[thread overview]
Message-ID: <20210715105223.30463-1-mwilck@suse.com> (raw)

From: Martin Wilck <mwilck@suse.com>

Current libmultipath contains a lot of code writing text to pre-allocated
string buffers of fixed size while trying to avoid buffer overflows
or truncation of the output string. This code is, at least in part,
hard to verify and hard to read, as buffer sizes and positions need to
be checked after every additon of text. It requires either allocating large
buffers in advance, or risking to loose data, or re-allocating buffers
and starting over again.

This patch set changes this by switching almost entirely to text buffers
that are dynamically allocated and grow automatically as text is added.

The first patch, which was the initial motivation of the set, doesn't
need growing buffers, it just overcomes the PARAMS_SIZE limitation by
copying data from libdm. However if the PARAMS_SIZE limitation was dropped
when reading from the kernel, it made sense to drop it for constructing
PARAMS, too. This motivated the implementation of the strbuf API (patch 2),
and using it in assemble_map (patch 3). The API was then consequently used
elswhere in libmultipath too, where strings are built up in pieces (patch 4,
5, 8, 9). These additional changes are huge because of large number of
affected "snprint_xyz" functions and respective function calls, but most
of the changes are mechanical and follow always the same overall idea.

Finally, patch 6 and 7 are cleanups which I found necessary while working
on print.c.

Regression testing: added a test suite for strbuf itself, made sure
all tests cases (which involve quite a bit of printf_xyz() functions) still
pass, and compared output of lots of interactive multipathd and multipath
commands before and after the change.

Note that "multipathd show config" / "multipath -t" will show some minor
differences in the output, because the current code omits quotes for some
output value fields. With this patch set, all string values are quoted.

The patch set is available also on my "tip" branch:
https://github.com/openSUSE/multipath-tools/tree/tip.

Martin Wilck (9):
  libmultipath: variable-size parameters in dm_get_map()
  libmultipath: strbuf: simple api for growing string buffers
  libmultipath: variable-size parameters in assemble_map()
  libmultipath: use strbuf in dict.c
  libmultipath: use strbuf in print.c
  libmultipath: print.c: fail hard if keywords are not found
  libmultipath: print.h: move macros to print.c
  libmultipath: use strbuf in alias.c.
  multipathd: use strbuf in cli.c

 libmultipath/Makefile                    |    2 +-
 libmultipath/alias.c                     |   84 +-
 libmultipath/blacklist.c                 |   13 +-
 libmultipath/configure.c                 |   18 +-
 libmultipath/configure.h                 |    3 +-
 libmultipath/devmapper.c                 |   44 +-
 libmultipath/devmapper.h                 |    4 +-
 libmultipath/dict.c                      |  313 ++--
 libmultipath/dict.h                      |   19 +-
 libmultipath/discovery.c                 |   13 +-
 libmultipath/dmparser.c                  |   47 +-
 libmultipath/dmparser.h                  |    2 +-
 libmultipath/foreign.c                   |   78 +-
 libmultipath/foreign.h                   |   13 +-
 libmultipath/foreign/nvme.c              |  100 +-
 libmultipath/generic.c                   |   26 +-
 libmultipath/generic.h                   |   17 +-
 libmultipath/libmultipath.version        |   21 +-
 libmultipath/parser.c                    |   50 +-
 libmultipath/parser.h                    |   17 +-
 libmultipath/print.c                     | 1839 ++++++++++------------
 libmultipath/print.h                     |  131 +-
 libmultipath/prioritizers/weightedpath.c |   71 +-
 libmultipath/propsel.c                   |  147 +-
 libmultipath/strbuf.c                    |  207 +++
 libmultipath/strbuf.h                    |  168 ++
 libmultipath/structs.h                   |    1 -
 libmultipath/structs_vec.c               |   11 +-
 libmultipath/util.c                      |    5 +
 libmultipath/util.h                      |    1 +
 multipathd/cli.c                         |   94 +-
 multipathd/cli_handlers.c                |  349 ++--
 multipathd/main.c                        |   20 +-
 tests/Makefile                           |    3 +-
 tests/alias.c                            |   41 +-
 tests/strbuf.c                           |  412 +++++
 36 files changed, 2312 insertions(+), 2072 deletions(-)
 create mode 100644 libmultipath/strbuf.c
 create mode 100644 libmultipath/strbuf.h
 create mode 100644 tests/strbuf.c

-- 
2.32.0


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


             reply	other threads:[~2021-07-15 10:54 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-15 10:52 mwilck [this message]
2021-07-15 10:52 ` [dm-devel] [PATCH 1/9] libmultipath: variable-size parameters in dm_get_map() mwilck
2021-07-26 22:17   ` Benjamin Marzinski
2021-08-11 14:18     ` Martin Wilck
2021-07-15 10:52 ` [dm-devel] [PATCH 2/9] libmultipath: strbuf: simple api for growing string buffers mwilck
2021-07-27  4:54   ` Benjamin Marzinski
2021-08-11 15:03     ` Martin Wilck
2021-07-15 10:52 ` [dm-devel] [PATCH 3/9] libmultipath: variable-size parameters in assemble_map() mwilck
2021-07-28 15:54   ` Benjamin Marzinski
2021-08-11 15:15     ` Martin Wilck
2021-07-15 10:52 ` [dm-devel] [PATCH 4/9] libmultipath: use strbuf in dict.c mwilck
2021-07-28 19:03   ` Benjamin Marzinski
2021-08-11 15:27     ` Martin Wilck
2021-08-30 18:23       ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 5/9] libmultipath: use strbuf in print.c mwilck
2021-07-29  0:00   ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 6/9] libmultipath: print.c: fail hard if keywords are not found mwilck
2021-07-29  2:36   ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 7/9] libmultipath: print.h: move macros to print.c mwilck
2021-07-29  2:36   ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 8/9] libmultipath: use strbuf in alias.c mwilck
2021-07-29 15:22   ` Benjamin Marzinski
2021-07-15 10:52 ` [dm-devel] [PATCH 9/9] multipathd: use strbuf in cli.c mwilck
2021-07-29 15:46   ` Benjamin Marzinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210715105223.30463-1-mwilck@suse.com \
    --to=mwilck@suse.com \
    --cc=bmarzins@redhat.com \
    --cc=christophe.varoqui@opensvc.com \
    --cc=dm-devel@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).