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

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

[Girish Joshi]

In argp-help.c `char *vt` is being initialized only once. It needs to
be initialized for every child in the doc and it needs to be printed
only in two cases.
1. There are no children and the doc does not starts with '\v'.
2. Argument `first_only` to the function `argp_doc` is false and the
complete doc needs to be printed.

diff --git a/argp/argp-help.c b/argp/argp-help.c
index 3b1727c4aa..ee4d247824 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;
     }
@@ -1498,8 +1499,11 @@ 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);
+      else{
+        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');

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

[Florian Weimer]

* Girish Joshi:

> In argp-help.c `char *vt` is being initialized only once. It needs to
> be initialized for every child in the doc and it needs to be printed
> only in two cases.

> 1. There are no children and the doc does not starts with '\v'.
> 2. Argument `first_only` to the function `argp_doc` is false and the
> complete doc needs to be printed.

I assume this bug user-visible, so it should have a bug in Bugzilla
<https://sourceware.org/bugzilla/>.  Would you please file one?

Could you write a test case for this?  Or at least post how to reproduce
the issue in a minimal program?

Thanks,
Florian

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

[Girish Joshi]

Hi Florian,

> I assume this bug user-visible, so it should have a bug in Bugzilla
> <https://sourceware.org/bugzilla/>.  Would you please file one?
>
Here[1] is a Bugzilla entry for this.

> Could you write a test case for this?  Or at least post how to reproduce
> the issue in a minimal program?
>
It can be reproduced with 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);
}

the output of the above code looks like:

this is post_doc

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

this is post_doc

In this case the doc is being printed two times.
As mentioned in the comment by Kenny Stauffer on Bugzilla the first
occurrence of `this is post_doc` is erroneous.

If we change the doc[] in the above code to something else like

static char doc[] = "this is pre_doc\vthis is post_doc";

We get the output like:

this is pre_doc

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

this is post_doc

[1]: https://sourceware.org/bugzilla/show_bug.cgi?id=19038

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

[Girish Joshi]

Could someone please review this patch?
Thanks.

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

[Adhemerval Zanella]

On 30/05/2019 05:14, Girish Joshi wrote:
> Could someone please review this patch?
> Thanks.
> 

My understanding is Florian has asked if you could write a testcase
using glibc libsupport and resend the patch along with the testcase.
It will also need a ChangeLog entry.

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

[Girish Joshi]

Hello,
Following is the patch with ChangeLog entry and a test case.
Please let me know if any changes are needed in it.

diff --git a/ChangeLog b/ChangeLog
index fbe5cd8648..7b11db3127 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2019-07-04  Girish Joshi  <girish946@gmail.com>
+
+    [BZ #19038]
+    * argp/argp-help.c: Added validation for printing pre and post doc.
+    * argp/tst-argp-help.c: Added testcase for docstring help/usage.
+
 2019-07-04  Andreas Schwab  <schwab@suse.de>

     [BZ #24484]
diff --git a/argp/Makefile b/argp/Makefile
index d5c2d77658..073ea3049e 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-argp-help

 CFLAGS-argp-help.c += $(uses-callbacks) -fexceptions
 CFLAGS-argp-parse.c += $(uses-callbacks)
@@ -35,5 +35,6 @@ CFLAGS-argp-fmtstream.c += -fexceptions

 bug-argp1-ARGS = -- --help
 bug-argp2-ARGS = -- -d 111 --dstaddr 222 -p 333 --peer 444
+tst-argp-help-ARGS = -- --help

 include ../Rules
diff --git a/argp/argp-help.c b/argp/argp-help.c
index 3b1727c4aa..ee4d247824 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;
     }
@@ -1498,8 +1499,11 @@ 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);
+      else{
+        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-argp-help.c b/argp/tst-argp-help.c
new file mode 100644
index 0000000000..7abb97f8df
--- /dev/null
+++ b/argp/tst-argp-help.c
@@ -0,0 +1,39 @@
+/* Copyright (C) 2002-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Ulrich Drepper <drepper@redhat.com>, 2002.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <argp.h>
+
+static int
+do_test (void)
+{
+
+  int argc = 2;
+  char *argv[3] = { (char *) "tst-argp-help", (char *) "--help", NULL };
+  int remaining;
+
+  static char doc[] = "\vthis is post_doc";
+  static struct argp argp = {NULL, NULL, NULL, doc};
+
+  /* Parse and process arguments.  */
+  argp_parse(&argp, argc, argv, 0, &remaining, NULL);
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"

Thanks.

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

[Girish Joshi]

My bad, I had copied the license part from another file and did not
change the Email-Id and author name.
> Following is the patch with ChangeLog entry and a test case.
> Please let me know if any changes are needed in it.

diff --git a/ChangeLog b/ChangeLog
index fbe5cd8648..7b11db3127 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2019-07-04  Girish Joshi  <girish946@gmail.com>
+
+    [BZ #19038]
+    * argp/argp-help.c: Added validation for printing pre and post doc.
+    * argp/tst-argp-help.c: Added testcase for docstring help/usage.
+
 2019-07-04  Andreas Schwab  <schwab@suse.de>

     [BZ #24484]
diff --git a/argp/Makefile b/argp/Makefile
index d5c2d77658..073ea3049e 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-argp-help

 CFLAGS-argp-help.c += $(uses-callbacks) -fexceptions
 CFLAGS-argp-parse.c += $(uses-callbacks)
@@ -35,5 +35,6 @@ CFLAGS-argp-fmtstream.c += -fexceptions

 bug-argp1-ARGS = -- --help
 bug-argp2-ARGS = -- -d 111 --dstaddr 222 -p 333 --peer 444
+tst-argp-help-ARGS = -- --help

 include ../Rules
diff --git a/argp/argp-help.c b/argp/argp-help.c
index 3b1727c4aa..ee4d247824 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;
     }
@@ -1498,8 +1499,11 @@ 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);
+      else{
+        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-argp-help.c b/argp/tst-argp-help.c
new file mode 100644
index 0000000000..3bfeb0219c
--- /dev/null
+++ b/argp/tst-argp-help.c
@@ -0,0 +1,39 @@
+/* Copyright (C) 2002-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by Girish Joshi <girish946@gmail.com>, 2019.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library 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
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <argp.h>
+
+static int
+do_test (void)
+{
+
+  int argc = 2;
+  char *argv[3] = { (char *) "tst-argp-help", (char *) "--help", NULL };
+  int remaining;
+
+  static char doc[] = "\vthis is post_doc";
+  static struct argp argp = {NULL, NULL, NULL, doc};
+
+  /* Parse and process arguments.  */
+  argp_parse(&argp, argc, argv, 0, &remaining, NULL);
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"

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

[Joseph Myers]

On Fri, 5 Jul 2019, Girish Joshi wrote:

> diff --git a/argp/tst-argp-help.c b/argp/tst-argp-help.c
> new file mode 100644
> index 0000000000..3bfeb0219c
> --- /dev/null
> +++ b/argp/tst-argp-help.c
> @@ -0,0 +1,39 @@
> +/* Copyright (C) 2002-2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.

Are you sure this has copyrightable content dating from 2002?

> +   Contributed by Girish Joshi <girish946@gmail.com>, 2019.

We don't put "Contributed by" in any new files, and haven't for several 
years.

> +  /* Parse and process arguments.  */
> +  argp_parse(&argp, argc, argv, 0, &remaining, NULL);

Please use GNU style, i.e. space before '('.  There are also coding style 
issues in the changes to argp-help.c (braces in the wrong place).

I'm not clear how this test verifies that the results are as expected, 
i.e. how it ensures that, if the change to argp-help.c is not present, the 
test FAILs.

I'm also not clear what the actual issue being fixed is.  Please make sure 
to include a git-style commit message in any patch submission that 
provides a self-contained explanation of the problem and how it is fixed.

> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"

New tests should not use the old-style test-skeleton.c.  See 
support/README-testing.c.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Girish Joshi]

Thanks Joseph for the review.
Here is a patch with the required changes.
Please let me know if more changes are needed in it.

argp/argp-help.c: added validation for leading \v in the doc string.
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  variable  is being
initialized only once, which causes the whole documentation to be printed if
there is a leading '\v' in the doc string.

Implementation:
It needs to be initialized for every child in the doc and it needs to be printed
only in two cases.
1. There are no children and the doc does not starts with '\v'.
2. Argument  to the function  is false and the
complete doc needs to be printed.
---
 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 c97e4c307c..8206da8cd8 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 85f5792bfe..68422127d2 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..997072ba7d
--- /dev/null
+++ b/argp/tst-argp3.c
@@ -0,0 +1,68 @@
+/* Test for argparse with leading '\v' in the doc string.
+  Copyright (C) 2019 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.0


Girish Joshi
girishjoshi.io

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

[Girish Joshi]

Could anyone please review this patch?
Thanks.

Girish Joshi
girishjoshi.io

On Mon, Nov 25, 2019 at 11:38 PM Girish Joshi <girish946@gmail.com> wrote:
>
> Thanks Joseph for the review.
> Here is a patch with the required changes.
> Please let me know if more changes are needed in it.
>
> argp/argp-help.c: added validation for leading \v in the doc string.
> 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  variable  is being
> initialized only once, which causes the whole documentation to be printed if
> there is a leading '\v' in the doc string.
>
> Implementation:
> It needs to be initialized for every child in the doc and it needs to be printed
> only in two cases.
> 1. There are no children and the doc does not starts with '\v'.
> 2. Argument  to the function  is false and the
> complete doc needs to be printed.
> ---
>  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 c97e4c307c..8206da8cd8 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 85f5792bfe..68422127d2 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..997072ba7d
> --- /dev/null
> +++ b/argp/tst-argp3.c
> @@ -0,0 +1,68 @@
> +/* Test for argparse with leading '\v' in the doc string.
> +  Copyright (C) 2019 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.0
>
>
> Girish Joshi
> girishjoshi.io

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

[Carlos O'Donell]

On 11/30/19 9:47 AM, Girish Joshi wrote:
> Could anyone please review this patch?
Do you have copyright assignment with the FSF for your glibc work?

Please have a look through:
https://sourceware.org/glibc/wiki/Contribution%20checklist

If you have any questions I'm happy to help.

-- 
Cheers,
Carlos.

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

[Girish Joshi]

Hello Carlos

> Do you have copyright assignment with the FSF for your glibc work?

Yes, now I have it.

Girish Joshi
girishjoshi.io

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

[Girish Joshi]

Hello,

If this patch is good enough, could someone please commit it?
Thanks.

On Mon, Nov 25, 2019 at 11:38 PM Girish Joshi <girish946@gmail.com> wrote:
>
> Thanks Joseph for the review.
> Here is a patch with the required changes.
> Please let me know if more changes are needed in it.
>
> argp/argp-help.c: added validation for leading \v in the doc string.
> 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  variable  is being
> initialized only once, which causes the whole documentation to be printed if
> there is a leading '\v' in the doc string.
>
> Implementation:
> It needs to be initialized for every child in the doc and it needs to be printed
> only in two cases.
> 1. There are no children and the doc does not starts with '\v'.
> 2. Argument  to the function  is false and the
> complete doc needs to be printed.
> ---
>  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 c97e4c307c..8206da8cd8 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 85f5792bfe..68422127d2 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..997072ba7d
> --- /dev/null
> +++ b/argp/tst-argp3.c
> @@ -0,0 +1,68 @@
> +/* Test for argparse with leading '\v' in the doc string.
> +  Copyright (C) 2019 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.0
>


Girish Joshi
girishjoshi.io

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

[Carlos O'Donell]

On 1/20/20 12:21 PM, Girish Joshi wrote:
> If this patch is good enough, could someone please commit it?
> Thanks.

The master branch is currently frozen for the 2.31 release so we'll have to wait
for master to reopens for active development but we can review this patch before
that time.

I haven't reviewed your patch yet and many of us are focused on release testing
at this point in time.

My suggestion is to repost the patch libc-alpha, clearly indicate your current
copyright status (which is that you have assignment), and the results of your
testing etc.

I would structure your patch as if it were a full and completely git commit
and post what is effetively a 'git format-patch HEAD~1' patch which includes
the commit message for review. This way the reviewer can review all parts of
the change.

In summary:
- Repost patch
  - State clarified copyright status.
  - Use 'git format-patch HEAD~1' to get a commit message and patch for review.
  - Make testing results clear.

> On Mon, Nov 25, 2019 at 11:38 PM Girish Joshi <girish946@gmail.com> wrote:
>>
>> Thanks Joseph for the review.
>> Here is a patch with the required changes.
>> Please let me know if more changes are needed in it.
>>
>> argp/argp-help.c: added validation for leading \v in the doc string.
>> 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  variable  is being
>> initialized only once, which causes the whole documentation to be printed if
>> there is a leading '\v' in the doc string.
>>
>> Implementation:
>> It needs to be initialized for every child in the doc and it needs to be printed
>> only in two cases.
>> 1. There are no children and the doc does not starts with '\v'.
>> 2. Argument  to the function  is false and the
>> complete doc needs to be printed.
>> ---
>>  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 c97e4c307c..8206da8cd8 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 85f5792bfe..68422127d2 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..997072ba7d
>> --- /dev/null
>> +++ b/argp/tst-argp3.c
>> @@ -0,0 +1,68 @@
>> +/* Test for argparse with leading '\v' in the doc string.
>> +  Copyright (C) 2019 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.0
>>
> 
> 
> Girish Joshi
> girishjoshi.io
> 


-- 
Cheers,
Carlos.

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

[Girish Joshi]

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

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

[Girish Joshi]

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

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

[Girish Joshi]

Hello,
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

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

[Girish Joshi via Libc-alpha]

Hello,
Can someone please review this patch?
Thanks.

Girish Joshi

On Tue, Mar 3, 2020 at 11:43 PM Girish Joshi <girish946@gmail.com> wrote:
>
> Hello,
> 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

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

[DJ Delorie via Libc-alpha]

Girish Joshi <girish946@gmail.com> writes:
> +        if ((!vt && !child) || (text == inp_text && !first_only))

I stared at this code for a long time, and I'm still not sure I
understand it.  I think that means the code is wrong, even if it does
the right thing.

The original bug has the root cause:

 "When processing what should come before the '\v', this section sets
  inp_text = doc and inp_text_limit = 0. That value of inp_text_limit is
  interpreted later to mean "print the whole string."

Would the logic be easier to follow if you just initialized
inp_text_limit to -1 and have -1 mean "print the whole string"?  That
way a value of zero is unambiguously an empty string.

You'd end up with code like this:

>   size_t inp_text_limit = -1;

>   inp_text_limit = (!post && vt) ? (vt - doc) : -1;

>   if (text == inp_text && inp_text_limit != -1)

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

[Girish Joshi via Libc-alpha]

Thanks DJ for the review.

On Sat, Mar 21, 2020 at 7:33 AM DJ Delorie <dj@redhat.com> wrote:
>
> Girish Joshi <girish946@gmail.com> writes:
> > +        if ((!vt && !child) || (text == inp_text && !first_only))
>
> I stared at this code for a long time, and I'm still not sure I
> understand it.  I think that means the code is wrong, even if it does
> the right thing.
>
> The original bug has the root cause:
>
>  "When processing what should come before the '\v', this section sets
>   inp_text = doc and inp_text_limit = 0. That value of inp_text_limit is
>   interpreted later to mean "print the whole string."
>
> Would the logic be easier to follow if you just initialized
> inp_text_limit to -1 and have -1 mean "print the whole string"?  That
> way a value of zero is unambiguously an empty string.
>

I see your point.

> You'd end up with code like this:
>
> >   size_t inp_text_limit = -1;
>
> >   inp_text_limit = (!post && vt) ? (vt - doc) : -1;
>
> >   if (text == inp_text && inp_text_limit != -1)
>

I tried this, it does work properly.
I'll create and post a patch with these changes tomorrow.
Thanks again for the input.

Girish Joshi

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

[Girish Joshi via Libc-alpha]

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


Girish Joshi
girishjoshi.io

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

[Girish Joshi via Libc-alpha]

Ping.

Girish Joshi

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
>
>
> Girish Joshi
> girishjoshi.io

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

[Girish Joshi via Libc-alpha]

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

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

[Florian Weimer via Libc-alpha]

* Girish Joshi via Libc-alpha:

> 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 \

This patch is malformed.  Please try reposting it as an attachment.
(I probably will not have time to look at this patch.)

Thanks,
Florian

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

[Girish Joshi via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 114 bytes --]

> This patch is malformed.  Please try reposting it as an attachment.

reposting the patch.
Thanks.

Girish Joshi

[-- Attachment #2: 0001-argp-argp-help.c-Corrected-the-default-value-and-usa.patch --]
[-- Type: text/x-patch, Size: 6039 bytes --]

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

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

[Zack Weinberg]

I wanted to review this patch, but I don't know enough about argp to
do it, and I don't have time today to learn how it works.

argp is shared with gnulib ( https://www.gnu.org/software/gnulib/ ).
I would like to suggest that you post this patch to
bug-gnulib@gnu.org.  (Despite the name, this is the appropriate list
to send gnulib patches to.)  There may be people reading that list who
can review your patch, and their review will be good enough for us as
well.

zw

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

[Girish Joshi via Libc-alpha]

[-- Attachment #1: Type: text/plain, Size: 345 bytes --]

Hi,

For a long time I could not work on this patch.
Recently I had sent the patch on gnulib mailing list, but did not get
a reply yet.
I'll send another follow up mail over there also.
Also I saw a few patches involving argp on the glibc mailing list.
So I'm sending this patch once again. Could someone please review it?
Thanks.

Girish Joshi

[-- Attachment #2: 0001-argp-argp-help.c-Corrected-the-default-value-and-usa.patch --]
[-- Type: text/x-patch, Size: 6039 bytes --]

From 4917277104171bca1fa8685e4188cb8780be7b3a Mon Sep 17 00:00:00 2001
From: Girish Joshi <girish946@gmail.com>
Date: Sun, 7 Feb 2021 23:58:36 +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 90023fba16..aa96a9c221 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 02afba2c46..bda572465a 100644
--- a/argp/argp-help.c
+++ b/argp/argp-help.c
@@ -1575,7 +1575,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;
 
@@ -1583,7 +1583,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;
@@ -1609,7 +1609,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.26.2