[2/4] parse-options: factor out parse_options_count()
diff mbox series

Message ID c729aaab-68d7-9cfa-8981-97eaa72a5ebe@web.de
State New
Headers show
Series
  • parse-options: simplify parse_options_concat() and parse_options_dup()
Related show

Commit Message

René Scharfe Feb. 9, 2020, 3:56 p.m. UTC
Add a helper function to count the number of options (excluding the
final OPT_END()) and use it to simplify parse_options_dup() and
parse_options_concat().

Signed-off-by: René Scharfe <l.s.r@web.de>
---
 parse-options-cb.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

--
2.25.0

Comments

Eric Sunshine Feb. 9, 2020, 5:56 p.m. UTC | #1
On Sun, Feb 9, 2020 at 10:56 AM René Scharfe <l.s.r@web.de> wrote:
> Add a helper function to count the number of options (excluding the
> final OPT_END()) and use it to simplify parse_options_dup() and
> parse_options_concat().
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> diff --git a/parse-options-cb.c b/parse-options-cb.c
> @@ -159,16 +159,20 @@ int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
>  struct option *parse_options_dup(const struct option *o)
>  {
>         const struct option *orig = o;
>         struct option *opts;
> -       int nr = 0;
> -
> -       while (o && o->type != OPTION_END) {
> -               nr++;
> -               o++;
> -       }
> +       size_t nr = parse_options_count(o);
>
>         ALLOC_ARRAY(opts, nr + 1);
>         COPY_ARRAY(opts, orig, nr);

This could use a little more cleanup. After this change, 'o' is never
again consulted or changed, and 'orig' points at the original value of
'o', which means 'o' and 'orig' have the same value now always.
Therefore, the additional cleanup would be to drop the declaration and
assignment of 'orig' and reference 'o' in COPY_ARRAY() rather than
'orig'.
René Scharfe Feb. 9, 2020, 6:36 p.m. UTC | #2
Am 09.02.20 um 18:56 schrieb Eric Sunshine:
> On Sun, Feb 9, 2020 at 10:56 AM René Scharfe <l.s.r@web.de> wrote:
>> Add a helper function to count the number of options (excluding the
>> final OPT_END()) and use it to simplify parse_options_dup() and
>> parse_options_concat().
>>
>> Signed-off-by: René Scharfe <l.s.r@web.de>
>> ---
>> diff --git a/parse-options-cb.c b/parse-options-cb.c
>> @@ -159,16 +159,20 @@ int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
>>  struct option *parse_options_dup(const struct option *o)
>>  {
>>         const struct option *orig = o;
>>         struct option *opts;
>> -       int nr = 0;
>> -
>> -       while (o && o->type != OPTION_END) {
>> -               nr++;
>> -               o++;
>> -       }
>> +       size_t nr = parse_options_count(o);
>>
>>         ALLOC_ARRAY(opts, nr + 1);
>>         COPY_ARRAY(opts, orig, nr);
>
> This could use a little more cleanup. After this change, 'o' is never
> again consulted or changed, and 'orig' points at the original value of
> 'o', which means 'o' and 'orig' have the same value now always.
> Therefore, the additional cleanup would be to drop the declaration and
> assignment of 'orig' and reference 'o' in COPY_ARRAY() rather than
> 'orig'.

True, but that would go beyond the purpose of this patch, which is to
extract a count function.  While it enables the cleanup you mentioned,
the latter should go into its own patch.  The last one in the series
takes care of it.

René

Patch
diff mbox series

diff --git a/parse-options-cb.c b/parse-options-cb.c
index 012e048856..db6f666ef7 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -159,16 +159,20 @@  int parse_opt_tertiary(const struct option *opt, const char *arg, int unset)
 	return 0;
 }

+static size_t parse_options_count(const struct option *opt)
+{
+	size_t n = 0;
+
+	for (; opt && opt->type != OPTION_END; opt++)
+		n++;
+	return n;
+}
+
 struct option *parse_options_dup(const struct option *o)
 {
 	const struct option *orig = o;
 	struct option *opts;
-	int nr = 0;
-
-	while (o && o->type != OPTION_END) {
-		nr++;
-		o++;
-	}
+	size_t nr = parse_options_count(o);

 	ALLOC_ARRAY(opts, nr + 1);
 	COPY_ARRAY(opts, orig, nr);
@@ -180,12 +184,8 @@  struct option *parse_options_dup(const struct option *o)
 struct option *parse_options_concat(struct option *a, struct option *b)
 {
 	struct option *ret;
-	size_t i, a_len = 0, b_len = 0;
-
-	for (i = 0; a[i].type != OPTION_END; i++)
-		a_len++;
-	for (i = 0; b[i].type != OPTION_END; i++)
-		b_len++;
+	size_t a_len = parse_options_count(a);
+	size_t b_len = parse_options_count(b);

 	ALLOC_ARRAY(ret, st_add3(a_len, b_len, 1));
 	COPY_ARRAY(ret, a, a_len);