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

[Girish Joshi]

Could someone please review this patch?

On Fri, May 8, 2020 at 1:23 AM Girish Joshi <girish946@gmail.com> wrote:
>
> Posting the corrected patch.
>
> From 47f3274930fa57b238a182ea92f64117bae76df0 Mon Sep 17 00:00:00 2001
> From: Girish Joshi <girish946@gmail.com>
> Date: Thu, 7 May 2020 12:53:06 +0530
> Subject: [PATCH] argp/argp-help.c: Corrected the default value and usage for
>  inp_text_limit in argp_doc(). argp/tst-argp3.c: added test case when the doc
>  string contains leading \v. argp/Makefile: added tst-argp3 to tests
>
> Overview:
> argp.doc prints incorrectly when it starts with '\v'.
> In argp-help.c in the function argp_doc() variable inp_text_limit is reset to 0
> if the doc string starts with '\v'. Which causes the whole doc string to be
> printed in the case of pre documentation, because of initialization of inp_text
> and inp_text_limit
>
>     inp_text = post ? (vt ? vt + 1 : 0) : doc;
>     inp_text_limit = (!post && vt) ? (vt - doc) : 0;
>
> and the condition where the doc string is printed.
>
>     if (text == inp_text && inp_text_limit)
>       __argp_fmtstream_write (stream, inp_text, inp_text_limit);
>
> So for the following code
>
>     #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);
>     }
>
> the output is
>
>     $ 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 bugzilla entry the first occurrence of
> "this is post_doc" is erroneous as it is the pre doc and there is nothing
> in the doc string in predoc section.
>
> Implementation:
> Reset the value of inp_text_limit to -1 if the doc string starts with '\v'.
> Modify the condition for printing the complete doc string with validation for
> inp_text_limit variable which looks like.
>
>     if (text == inp_text && inp_text_limit != -1)
>       __argp_fmtstream_write (stream, inp_text, inp_text_limit);
>
> after this 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
> ---
>  argp/Makefile    |  2 +-
>  argp/argp-help.c |  6 ++---
>  argp/tst-argp3.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 72 insertions(+), 4 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..2aadbcb09e 100644
> --- a/argp/argp-help.c
> +++ b/argp/argp-help.c
> @@ -1462,7 +1462,7 @@ argp_doc (const struct argp *argp, const struct
> argp_state *state,
>    const char *inp_text;
>    void *input = 0;
>    int anything = 0;
> -  size_t inp_text_limit = 0;
> +  size_t inp_text_limit = -1;
>    const char *doc = dgettext (argp->argp_domain, argp->doc);
>    const struct argp_child *child = argp->children;
>
> @@ -1470,7 +1470,7 @@ argp_doc (const struct argp *argp, const struct
> argp_state *state,
>      {
>        char *vt = strchr (doc, '\v');
>        inp_text = post ? (vt ? vt + 1 : 0) : doc;
> -      inp_text_limit = (!post && vt) ? (vt - doc) : 0;
> +      inp_text_limit = (!post && vt) ? (vt - doc) : -1;
>      }
>    else
>      inp_text = 0;
> @@ -1496,7 +1496,7 @@ argp_doc (const struct argp *argp, const struct
> argp_state *state,
>        if (pre_blank)
>      __argp_fmtstream_putc (stream, '\n');
>
> -      if (text == inp_text && inp_text_limit)
> +      if (text == inp_text && inp_text_limit != -1)
>      __argp_fmtstream_write (stream, inp_text, inp_text_limit);
>        else
>      __argp_fmtstream_puts (stream, text);
> 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