[PATCH] getline.3: wfix.

[PATCH] getline.3: wfix.

[enh]

Clarify in the RETURN VALUE section that it's necessary to free the
buffer, since at least one person missed this up at the start of the
DESCRIPTION.

Signed-off-by: Elliott Hughes <enh@google.com>
---
 man3/getline.3 | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/man3/getline.3 b/man3/getline.3
index 8b7357825..bad30956c 100644
--- a/man3/getline.3
+++ b/man3/getline.3
@@ -99,6 +99,10 @@ condition).
 In the event of a failure,
 .I errno
 is set to indicate the error.
+If
+.I "*lineptr"
+was set to NULL before the call, then the buffer should be freed by the
+user program even on failure.
 .SH ERRORS
 .TP
 .B EINVAL
-- 
2.37.1.455.g008518b4e5-goog

Re: [PATCH] getline.3: wfix.

[G. Branden Robinson]

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

Hi Elliott,

At 2022-07-29T13:22:22-0700, enh wrote:
> Clarify in the RETURN VALUE section that it's necessary to free the
> buffer, since at least one person missed this up at the start of the
> DESCRIPTION.
[...]
> +If
> +.I "*lineptr"

The quotation marks are unnecessary; macro arguments are space-delimited
and you have none here.  (Even if you had multiple arguments, macros
like `B` and `I` treat them all the same.)

Regards,
Branden

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] getline.3: wfix.

[enh]

i actually just copy & pasted that from the top of the file :-)

On Fri, Jul 29, 2022 at 1:52 PM G. Branden Robinson
<g.branden.robinson@gmail.com> wrote:
>
> Hi Elliott,
>
> At 2022-07-29T13:22:22-0700, enh wrote:
> > Clarify in the RETURN VALUE section that it's necessary to free the
> > buffer, since at least one person missed this up at the start of the
> > DESCRIPTION.
> [...]
> > +If
> > +.I "*lineptr"
>
> The quotation marks are unnecessary; macro arguments are space-delimited
> and you have none here.  (Even if you had multiple arguments, macros
> like `B` and `I` treat them all the same.)
>
> Regards,
> Branden

Re: [PATCH] getline.3: wfix.

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1013 bytes --]

Hi enh,

On 7/29/22 22:22, enh wrote:
> Clarify in the RETURN VALUE section that it's necessary to free the
> buffer, since at least one person missed this up at the start of the
> DESCRIPTION.
> 
> Signed-off-by: Elliott Hughes <enh@google.com>
> ---
>   man3/getline.3 | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/man3/getline.3 b/man3/getline.3
> index 8b7357825..bad30956c 100644
> --- a/man3/getline.3
> +++ b/man3/getline.3
> @@ -99,6 +99,10 @@ condition).
>   In the event of a failure,
>   .I errno
>   is set to indicate the error.

.PP

Since the added text has nothing to do with the previous, let's separate 
them into separate paragraphs.

> +If
> +.I "*lineptr"

As Branden said, please remove the unnecessary quotes.

> +was set to NULL before the call, then the buffer should be freed by the
> +user program even on failure.
>   .SH ERRORS
>   .TP
>   .B EINVAL

Cheers,

Alex

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] getline.3: wfix.

[enh]

ack. i've fixed all the other instances of `.I "foo"` in that file too...

Date: Fri, 29 Jul 2022 13:13:10 -0700
Subject: [PATCH] getline.3: wfix.

Clarify in the RETURN VALUE section that it's necessary to free the
buffer, since at least one person missed this up at the start of the
DESCRIPTION.

Signed-off-by: Elliott Hughes <enh@google.com>
---
 man3/getline.3 | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/man3/getline.3 b/man3/getline.3
index 8b7357825..cb9e5b049 100644
--- a/man3/getline.3
+++ b/man3/getline.3
@@ -36,12 +36,12 @@ Feature Test Macro Requirements for glibc (see
 .BR getline ()
 reads an entire line from \fIstream\fP,
 storing the address of the buffer containing the text into
-.IR "*lineptr" .
+.IR *lineptr .
 The buffer is null-terminated and includes the newline character, if
 one was found.
 .PP
 If
-.I "*lineptr"
+.I *lineptr
 is set to NULL before the call, then
 .BR getline ()
 will allocate a buffer for storing the line.
@@ -52,26 +52,26 @@ failed.
 .PP
 Alternatively, before calling
 .BR getline (),
-.I "*lineptr"
+.I *lineptr
 can contain a pointer to a
 .BR malloc (3)\-allocated
 buffer
-.I "*n"
+.I *n
 bytes in size.
 If the buffer is not large enough to hold the line,
 .BR getline ()
 resizes it with
 .BR realloc (3),
 updating
-.I "*lineptr"
+.I *lineptr
 and
-.I "*n"
+.I *n
 as necessary.
 .PP
 In either case, on a successful call,
-.I "*lineptr"
+.I *lineptr
 and
-.I "*n"
+.I *n
 will be updated to reflect the buffer address and allocated size respectively.
 .PP
 .BR getdelim ()
@@ -99,6 +99,11 @@ condition).
 In the event of a failure,
 .I errno
 is set to indicate the error.
+.PP
+If
+.I *lineptr
+was set to NULL before the call, then the buffer should be freed by the
+user program even on failure.
 .SH ERRORS
 .TP
 .B EINVAL
-- 
2.37.1.455.g008518b4e5-goog

On Mon, Aug 1, 2022 at 5:54 AM Alejandro Colomar <alx.manpages@gmail.com> wrote:
>
> Hi enh,
>
> On 7/29/22 22:22, enh wrote:
> > Clarify in the RETURN VALUE section that it's necessary to free the
> > buffer, since at least one person missed this up at the start of the
> > DESCRIPTION.
> >
> > Signed-off-by: Elliott Hughes <enh@google.com>
> > ---
> >   man3/getline.3 | 4 ++++
> >   1 file changed, 4 insertions(+)
> >
> > diff --git a/man3/getline.3 b/man3/getline.3
> > index 8b7357825..bad30956c 100644
> > --- a/man3/getline.3
> > +++ b/man3/getline.3
> > @@ -99,6 +99,10 @@ condition).
> >   In the event of a failure,
> >   .I errno
> >   is set to indicate the error.
>
> .PP
>
> Since the added text has nothing to do with the previous, let's separate
> them into separate paragraphs.
>
> > +If
> > +.I "*lineptr"
>
> As Branden said, please remove the unnecessary quotes.
>
> > +was set to NULL before the call, then the buffer should be freed by the
> > +user program even on failure.
> >   .SH ERRORS
> >   .TP
> >   .B EINVAL
>
> Cheers,
>
> Alex
>
> --
> Alejandro Colomar
> <http://www.alejandro-colomar.es/>

italicizing pointer stars (was: [PATCH] getline.3: wfix.)

[G. Branden Robinson]

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

At the risk of opening another typographical can of worms...

At 2022-08-02T14:17:24-0700, enh wrote:
> diff --git a/man3/getline.3 b/man3/getline.3
> index 8b7357825..cb9e5b049 100644
> --- a/man3/getline.3
> +++ b/man3/getline.3
> @@ -36,12 +36,12 @@ Feature Test Macro Requirements for glibc (see
>  .BR getline ()
>  reads an entire line from \fIstream\fP,
>  storing the address of the buffer containing the text into
> -.IR "*lineptr" .
> +.IR *lineptr .
[several other instances in same page snipped]

I'm wondering if we should really be setting the dereferencing operator
in italics here.

In declarations, the star is part of the _type_, not the identifier.
Similarly, in expressions, the star is an operator, like "+", not part
of the identifier.

Before answering, consider also the common practice of "setting all
literals in bold".  I think this habit is over-applied, and thus I do
not articulate it in groff_man_style(7)[1].  Here, an insistence upon it
in the above-quoted case leads us straight into the three-font problem,
and an especially devilish case of it to boot.

We would need...

.BI * lineptr\c
\&.

...where our man page contributors find not only that they need to
apply the historically nigh-incomprehensible `\c` escape sequence,[2]
but also have to remember to put the ineffable `\&` escape sequence[3]
at the start of the line so that the period will be formatted (rather
than interpreted as the *roff control character).

If the consensus is that applying the "literals in bold" convention
would indeed be too painful, then I suggest adding a style note
regarding this point to man-pages(7), and explaining why it was adopted.

I can propose some language for that.

There _is_ another way to skin this cat.[4]

.B *\c
.IR lineptr .

Maybe that would be preferable.

Regards,
Branden

[1] https://git.savannah.gnu.org/cgit/groff.git/tree/tmac/groff_man.7.man.in
[2] I believe it is explained clearly here.
    https://git.savannah.gnu.org/cgit/groff.git/tree/doc/groff.texi#n9509
[3] An argument has been raging on the groff mailing list over the best
    nomenclature for `\&` for some time.
    https://savannah.gnu.org/bugs/index.php?62816
[4] I reject the use font selection escape sequences out of hand.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: italicizing pointer stars (was: [PATCH] getline.3: wfix.)

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1929 bytes --]

Hi Branden,

On 8/5/22 21:20, G. Branden Robinson wrote:
> At the risk of opening another typographical can of worms...
> 
> At 2022-08-02T14:17:24-0700, enh wrote:
>> diff --git a/man3/getline.3 b/man3/getline.3
>> index 8b7357825..cb9e5b049 100644
>> --- a/man3/getline.3
>> +++ b/man3/getline.3
>> @@ -36,12 +36,12 @@ Feature Test Macro Requirements for glibc (see
>>   .BR getline ()
>>   reads an entire line from \fIstream\fP,
>>   storing the address of the buffer containing the text into
>> -.IR "*lineptr" .
>> +.IR *lineptr .
> [several other instances in same page snipped]
> 
> I'm wondering if we should really be setting the dereferencing operator
> in italics here.
> 
> In declarations, the star is part of the _type_, not the identifier.
> Similarly, in expressions, the star is an operator, like "+", not part
> of the identifier.

Let me escape out of this a bit sideways :P

*foo, as a whole, is an expression, consisting of an operator and an 
identifier.  So I won't apply identifier rules for highlighting, but 
expression rules.  The rules for marking expressions up are:

man-pages(7):
    Formatting conventions (general)
        [...]

        Expressions,  if not written on a separate indented line,
        should be specified in italics.  Again, the use  of  non‐
        breaking  spaces  may be appropriate if the expression is
        inlined with normal text.


And the deep rationale why I would like to avoid having the star in a 
different font is that it could be confused with the glob-like 
expressions that we use for example to refer to SYS_* macros.  (And, 
BTW, I should apply some consistency fixes to those too, since they are 
in many cases highlighted in bold, as the rest of the identifier --and 
that is plain wrong--.)

Did I dodge the bullet?

Cheers,

Alex

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: italicizing pointer stars (was: [PATCH] getline.3: wfix.)

[G. Branden Robinson]

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

Hi Alex,

At 2022-08-05T21:40:06+0200, Alejandro Colomar wrote:
> On 8/5/22 21:20, G. Branden Robinson wrote:
> > I'm wondering if we should really be setting the dereferencing
> > operator in italics here.
> > 
> > In declarations, the star is part of the _type_, not the identifier.
> > Similarly, in expressions, the star is an operator, like "+", not
> > part of the identifier.
> 
> Let me escape out of this a bit sideways :P
> 
> *foo, as a whole, is an expression, consisting of an operator and an
> identifier.  So I won't apply identifier rules for highlighting, but
> expression rules.  The rules for marking expressions up are:
> 
> man-pages(7):
>    Formatting conventions (general)
>        [...]
> 
>        Expressions,  if not written on a separate indented line,
>        should be specified in italics.  Again, the use  of  non‐
>        breaking  spaces  may be appropriate if the expression is
>        inlined with normal text.

Ahhh.  I had just reviewed that page while pondering a bunch of
miscellaneous suggestions I might craft into a patch series, but the
foregoing utterly failed to sink in.

> And the deep rationale why I would like to avoid having the star in a
> different font is that it could be confused with the glob-like
> expressions that we use for example to refer to SYS_* macros.

That makes sense.  I think instead of adopting glob syntax, I would use
quotation, but quotation in man(7) is so awkward that many people act
like it's unavailable.  This is just one of many reasons I think man(7)
should have a `Q` macro to facilitate it.

> Did I dodge the bullet?

I think so.  :)

Regards,
Branden

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: italicizing pointer stars (was: [PATCH] getline.3: wfix.)

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 885 bytes --]

Hi Branden,

On 8/5/22 23:49, G. Branden Robinson wrote:
>> And the deep rationale why I would like to avoid having the star in a
>> different font is that it could be confused with the glob-like
>> expressions that we use for example to refer to SYS_* macros.
> 
> That makes sense.  I think instead of adopting glob syntax, I would use
> quotation, but quotation in man(7) is so awkward that many people act
> like it's unavailable.  This is just one of many reasons I think man(7)
> should have a `Q` macro to facilitate it.

What do you mean by quotation?  How would you transform the following 
(taken from clone(2)):

        #include <sys/syscall.h>    /* Definition of SYS_* constants */

> 
>> Did I dodge the bullet?
> 
> I think so.  :)

:)

Cheers,

Alex

> 
> Regards,
> Branden

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] getline.3: wfix.

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 2308 bytes --]

Hi Elliott,

On 8/2/22 23:17, enh wrote:
> ack. i've fixed all the other instances of `.I "foo"` in that file too...
> 
> Date: Fri, 29 Jul 2022 13:13:10 -0700
> Subject: [PATCH] getline.3: wfix.
> 
> Clarify in the RETURN VALUE section that it's necessary to free the
> buffer, since at least one person missed this up at the start of the
> DESCRIPTION.
> 
> Signed-off-by: Elliott Hughes <enh@google.com>

I broke your patch into two separate patches, and applied them both.

Thanks,

Alex

> ---
>   man3/getline.3 | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/man3/getline.3 b/man3/getline.3
> index 8b7357825..cb9e5b049 100644
> --- a/man3/getline.3
> +++ b/man3/getline.3
> @@ -36,12 +36,12 @@ Feature Test Macro Requirements for glibc (see
>   .BR getline ()
>   reads an entire line from \fIstream\fP,
>   storing the address of the buffer containing the text into
> -.IR "*lineptr" .
> +.IR *lineptr .
>   The buffer is null-terminated and includes the newline character, if
>   one was found.
>   .PP
>   If
> -.I "*lineptr"
> +.I *lineptr
>   is set to NULL before the call, then
>   .BR getline ()
>   will allocate a buffer for storing the line.
> @@ -52,26 +52,26 @@ failed.
>   .PP
>   Alternatively, before calling
>   .BR getline (),
> -.I "*lineptr"
> +.I *lineptr
>   can contain a pointer to a
>   .BR malloc (3)\-allocated
>   buffer
> -.I "*n"
> +.I *n
>   bytes in size.
>   If the buffer is not large enough to hold the line,
>   .BR getline ()
>   resizes it with
>   .BR realloc (3),
>   updating
> -.I "*lineptr"
> +.I *lineptr
>   and
> -.I "*n"
> +.I *n
>   as necessary.
>   .PP
>   In either case, on a successful call,
> -.I "*lineptr"
> +.I *lineptr
>   and
> -.I "*n"
> +.I *n
>   will be updated to reflect the buffer address and allocated size respectively.
>   .PP
>   .BR getdelim ()
> @@ -99,6 +99,11 @@ condition).
>   In the event of a failure,
>   .I errno
>   is set to indicate the error.
> +.PP
> +If
> +.I *lineptr
> +was set to NULL before the call, then the buffer should be freed by the
> +user program even on failure.
>   .SH ERRORS
>   .TP
>   .B EINVAL

-- 
Alejandro Colomar
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]