argp: argp.doc prints incorrectly when it starts with "\v" [BZ #19038]

  Thanks Carlos for the input,

Reposting this patch for BZ #19038, as now I have the copyright assignment
with FSF.

Here the bug is that; if doc string starts with `\v` then the whole doc string
is printed both before and after the options.

The bug can be reproduced using following code.

#include<stdlib.h>
#include<argp.h>

static char doc[] = "\vthis is post_doc";
static struct argp argp = {NULL, NULL, NULL, doc};

int main(int argc, char *args[]){
     argp_parse(&argp, argc, args, 0, 0, NULL);
}

Currently we get the output of this code as

$ argp-help --help
Usage: argp-help [OPTION...]

this is post_doc

  -?, --help                 Give this help list
      --usage                Give a short usage message

this is post_doc

As mentioned in the comment on bugzilla, the first occurrence of
"this is post_doc" is erroneous.

This is happening because in argp-help.c in the function argp_doc 'char *vt'
(which is responsible for determining the position for vertical tab in the
doc string) is being initialized only once,

It should be initialized for each child in the documentation.

The next thing is the complete doc string needs to be printed only if
1. If there is only one child in the doc string
   and the doc string does not start with '\v'.
2. The argument 'first_only' to the function 'argp_doc' is 0.

After applying the patch we get the output as following.

$ argp-help --help
Usage: argp-help [OPTION...]

  -?, --help                 Give this help list
      --usage                Give a short usage message

this is post_doc

A test case for this is also added in the patch.
Following is the patch for this bug.

From 36cfdeab88ec3ecfe971cd70c798a9615d6adc41 Mon Sep 17 00:00:00 2001
From: Girish Joshi <girish946@gmail.com>
Date: Thu, 6 Feb 2020 01:03:18 +0530
Subject: [PATCH] Fix argp.doc prints incorrectly when it starts with '\v' (Bug
 19038)

"char *vt" in the function "argp_doc" is initialized for every child in the doc.

The complete doc string is printed only in two cases.
1. There is only one child in the doc and the doc does not starts with '\v'.
2. Argument "first_only" to the function "argp_doc" is 0.

Added test argp/tst-argp3.c for this case.
---
 argp/Makefile    |  2 +-
 argp/argp-help.c | 10 +++++--
 argp/tst-argp3.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 3 deletions(-)
 create mode 100644 argp/tst-argp3.c
  

Hello,
Could someone please review this patch?

Thanks.

On Wed, Feb 12, 2020 at 12:10 AM Girish Joshi <girish946@gmail.com> wrote:
>
> Thanks Carlos for the input,
>
> Reposting this patch for BZ #19038, as now I have the copyright assignment
> with FSF.
>
> Here the bug is that; if doc string starts with `\v` then the whole doc string
> is printed both before and after the options.
>
> The bug can be reproduced using following code.
>
> #include<stdlib.h>
> #include<argp.h>
>
> static char doc[] = "\vthis is post_doc";
> static struct argp argp = {NULL, NULL, NULL, doc};
>
> int main(int argc, char *args[]){
>      argp_parse(&argp, argc, args, 0, 0, NULL);
> }
>
> Currently we get the output of this code as
>
> $ argp-help --help
> Usage: argp-help [OPTION...]
>
> this is post_doc
>
>   -?, --help                 Give this help list
>       --usage                Give a short usage message
>
> this is post_doc
>
> As mentioned in the comment on bugzilla, the first occurrence of
> "this is post_doc" is erroneous.
>
> This is happening because in argp-help.c in the function argp_doc 'char *vt'
> (which is responsible for determining the position for vertical tab in the
> doc string) is being initialized only once,
>
> It should be initialized for each child in the documentation.
>
> The next thing is the complete doc string needs to be printed only if
> 1. If there is only one child in the doc string
>    and the doc string does not start with '\v'.
> 2. The argument 'first_only' to the function 'argp_doc' is 0.
>
> After applying the patch we get the output as following.
>
> $ argp-help --help
> Usage: argp-help [OPTION...]
>
>   -?, --help                 Give this help list
>       --usage                Give a short usage message
>
> this is post_doc
>
> A test case for this is also added in the patch.
> Following is the patch for this bug.
>
> From 36cfdeab88ec3ecfe971cd70c798a9615d6adc41 Mon Sep 17 00:00:00 2001
> From: Girish Joshi <girish946@gmail.com>
> Date: Thu, 6 Feb 2020 01:03:18 +0530
> Subject: [PATCH] Fix argp.doc prints incorrectly when it starts with '\v' (Bug
>  19038)
>
> "char *vt" in the function "argp_doc" is initialized for every child in the doc.
>
> The complete doc string is printed only in two cases.
> 1. There is only one child in the doc and the doc does not starts with '\v'.
> 2. Argument "first_only" to the function "argp_doc" is 0.
>
> Added test argp/tst-argp3.c for this case.
> ---
>  argp/Makefile    |  2 +-
>  argp/argp-help.c | 10 +++++--
>  argp/tst-argp3.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 77 insertions(+), 3 deletions(-)
>  create mode 100644 argp/tst-argp3.c
>
> diff --git a/argp/Makefile b/argp/Makefile
> index 1f9b074bed..0c0270ffef 100644
> --- a/argp/Makefile
> +++ b/argp/Makefile
> @@ -27,7 +27,7 @@ routines    = $(addprefix argp-, ba fmtstream
> fs-xinl help parse pv \
>                       pvh xinl eexst)
>
>  tests        = argp-test tst-argp1 bug-argp1 tst-argp2 bug-argp2 \
> -          tst-ldbl-argp
> +          tst-ldbl-argp tst-argp3
>
>  CFLAGS-argp-help.c += $(uses-callbacks) -fexceptions
>  CFLAGS-argp-parse.c += $(uses-callbacks)
> diff --git a/argp/argp-help.c b/argp/argp-help.c
> index 2bcd6549fd..8d213e4586 100644
> --- a/argp/argp-help.c
> +++ b/argp/argp-help.c
> @@ -1465,10 +1465,11 @@ argp_doc (const struct argp *argp, const
> struct argp_state *state,
>    size_t inp_text_limit = 0;
>    const char *doc = dgettext (argp->argp_domain, argp->doc);
>    const struct argp_child *child = argp->children;
> +  char *vt = 0;
>
>    if (doc)
>      {
> -      char *vt = strchr (doc, '\v');
> +      vt = strchr (doc, '\v');
>        inp_text = post ? (vt ? vt + 1 : 0) : doc;
>        inp_text_limit = (!post && vt) ? (vt - doc) : 0;
>      }
> @@ -1499,7 +1500,12 @@ argp_doc (const struct argp *argp, const struct
> argp_state *state,
>        if (text == inp_text && inp_text_limit)
>      __argp_fmtstream_write (stream, inp_text, inp_text_limit);
>        else
> -    __argp_fmtstream_puts (stream, text);
> +      {
> +        if ((!vt && !child) || (text == inp_text && !first_only))
> +        {
> +          __argp_fmtstream_puts (stream, text);
> +        }
> +      }
>
>        if (__argp_fmtstream_point (stream) > __argp_fmtstream_lmargin (stream))
>      __argp_fmtstream_putc (stream, '\n');
> diff --git a/argp/tst-argp3.c b/argp/tst-argp3.c
> new file mode 100644
> index 0000000000..cfdace2574
> --- /dev/null
> +++ b/argp/tst-argp3.c
> @@ -0,0 +1,68 @@
> +/* Test for argparse with leading '\v' in the doc string.
> +  Copyright (C) 2020 Free Software Foundation, Inc.
> +
> +  This program is free software: you can redistribute it and/or modify
> +  it under the terms of the GNU General Public License as published by
> +  the Free Software Foundation; either version 3 of the License, or
> +  (at your option) any later version.
> +
> +  This program is distributed in the hope that it will be useful,
> +  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +  GNU General Public License for more details.
> +
> +  You should have received a copy of the GNU General Public License
> +  along with this program.  If not, see <https://www.gnu.org/licenses/>.*/
> +
> +
> +#include<stdlib.h>
> +#include<argp.h>
> +
> +#include <support/capture_subprocess.h>
> +#include <support/check.h>
> +
> +
> +static char expected_success[] = "Usage: arp [OPTION...]\n\
> +\n\
> +  -?, --help                 Give this help list\n\
> +      --usage                Give a short usage message\n\
> +\n\
> +this is post_doc\n\
> +";
> +char *argv[3] = { (char *) "arp", NULL, NULL };
> +
> +static void
> +do_test_call (void)
> +{
> +  static char doc[] = "\vthis is post_doc";
> +  static struct argp argp = {NULL, NULL, NULL, doc};
> +
> +  argp_parse (&argp, 2, argv, 0, 0, NULL);
> +}
> +
> +static int
> +do_one_test (const char *expected)
> +{
> +  struct support_capture_subprocess result;
> +  result = support_capture_subprocess ((void *) &do_test_call, NULL);
> +
> +  TEST_COMPARE_STRING (result.out.buffer, expected);
> +
> +  return 0;
> +}
> +
> +
> +static int
> +do_test (void)
> +{
> +  const char *argument = "--help";
> +  argv[1] = (char *)argument;
> +  // success condition
> +  do_one_test (expected_success);
> +  return 0;
> +}
> +
> +/* This file references do_test above and contains the definition of
> +   the main function.  */
> +#include <support/test-driver.c>
> +
> --
> 2.21.1
>
> Thanks.
> Girish Joshi
> girishjoshi.io