"WARNING: parenthesis in function name" correctness

"WARNING: parenthesis in function name" correctness

[Yuri Pankov]

I'm looking into fixing sbuf.9 which currently has somewhat unreadable:

.Ft typedef\ int ( sbuf_drain_func ) ( void\ *arg, const\ char\ *data, 
int\ len ) ;

...into somewhat nicer:

.Ft typedef int
.Fo (sbuf_drain_func)
.Fa "void *arg"
.Fa "const char *data"
.Fa "int len"
.Fc

...but that gives me a:

mandoc: share/man/man9/sbuf.9:69:5: WARNING: parenthesis in function 
name: (sbuf_drain_func)

Looking at the code, parenthesis are only allowed in the following form:

.Ft (*func_ptr)

...while here we don't have a "*" -- it's the way it's typedef'ed in the 
source, and I want to follow it, so I wonder if that check/warning is 
really useful.
--
 To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv

Re: "WARNING: parenthesis in function name" correctness

[Ingo Schwarze]

Hi Yuri,

Yuri Pankov wrote on Tue, Sep 10, 2019 at 11:20:24AM +0300:

> I'm looking into fixing sbuf.9 which currently has somewhat unreadable:
> 
> .Ft typedef\ int ( sbuf_drain_func ) ( void\ *arg, const\ char\ *data, 
> int\ len ) ;

Indeed, that is horrible and should be fixed.

> ...into somewhat nicer:
> 
> .Ft typedef int
> .Fo (sbuf_drain_func)
> .Fa "void *arg"
> .Fa "const char *data"
> .Fa "int len"
> .Fc
> 
> ...but that gives me a:
> 
> mandoc: share/man/man9/sbuf.9:69:5: WARNING: parenthesis in function 
> name: (sbuf_drain_func)
> 
> Looking at the code, parenthesis are only allowed in the following form:
> 
> .Ft (*func_ptr)

I think you mean .Fo/.Fn, not .Ft, right?

> ...while here we don't have a "*" -- it's the way it's typedef'ed in the 
> source, and I want to follow it, so I wonder if that check/warning is 
> really useful.

If i understand the C standard correctly (otherwise, please somebody
correct me!), we have:

  int (f)()    --  function returning int
  int (*fp)()  --  pointer to function returning int

In many situations, both can be used interchangeably, consider for
example C89 3.2.2.1: "A function designator is an expression that
has function type.  Except when it is the operand of the sizeof
operator or the unary & operator, a function designator with type
``function returning type'' is converted to an expression that has
type ``pointer to function returning type.''"

Then again, it seems for sizeof(), it may make a difference whether
a type is a function type or a function pointer type, and maybe there
are also other situations where that matters.

So i suspect you are right there are legitimate reasons for making
the difference in the documentation, too,
and saying ".Fo (sbuf_drain_func)" looks reasonable.
Do others agree?

I plan to suppress the warning for ".Fo (f)" just like for ".Fo (*f)"
in the future.  The likely typo that should be warned about
is ".Fo f()", and that warning will still work.

Yours,
  Ingo
--
 To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv

Re: "WARNING: parenthesis in function name" correctness

[Yuri Pankov]

Ingo Schwarze wrote:
> Hi Yuri,
> 
> Yuri Pankov wrote on Tue, Sep 10, 2019 at 11:20:24AM +0300:
> 
>> I'm looking into fixing sbuf.9 which currently has somewhat unreadable:
>>
>> .Ft typedef\ int ( sbuf_drain_func ) ( void\ *arg, const\ char\ *data,
>> int\ len ) ;
> 
> Indeed, that is horrible and should be fixed.
> 
>> ...into somewhat nicer:
>>
>> .Ft typedef int
>> .Fo (sbuf_drain_func)
>> .Fa "void *arg"
>> .Fa "const char *data"
>> .Fa "int len"
>> .Fc
>>
>> ...but that gives me a:
>>
>> mandoc: share/man/man9/sbuf.9:69:5: WARNING: parenthesis in function
>> name: (sbuf_drain_func)
>>
>> Looking at the code, parenthesis are only allowed in the following form:
>>
>> .Ft (*func_ptr)
> 
> I think you mean .Fo/.Fn, not .Ft, right?

Yes, I was thinking about .Fo, sorry.

>> ...while here we don't have a "*" -- it's the way it's typedef'ed in the
>> source, and I want to follow it, so I wonder if that check/warning is
>> really useful.
> 
> If i understand the C standard correctly (otherwise, please somebody
> correct me!), we have:
> 
>    int (f)()    --  function returning int
>    int (*fp)()  --  pointer to function returning int
> 
> In many situations, both can be used interchangeably, consider for
> example C89 3.2.2.1: "A function designator is an expression that
> has function type.  Except when it is the operand of the sizeof
> operator or the unary & operator, a function designator with type
> ``function returning type'' is converted to an expression that has
> type ``pointer to function returning type.''"
> 
> Then again, it seems for sizeof(), it may make a difference whether
> a type is a function type or a function pointer type, and maybe there
> are also other situations where that matters.
> 
> So i suspect you are right there are legitimate reasons for making
> the difference in the documentation, too,
> and saying ".Fo (sbuf_drain_func)" looks reasonable.
> Do others agree?
> 
> I plan to suppress the warning for ".Fo (f)" just like for ".Fo (*f)"
> in the future.  The likely typo that should be warned about
> is ".Fo f()", and that warning will still work.

Sounds good, thank you!
--
 To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv

Re: "WARNING: parenthesis in function name" correctness

[Ingo Schwarze]

Hi Yuri,

Yuri Pankov wrote on Wed, Sep 11, 2019 at 01:43:22AM +0300:
> Ingo Schwarze wrote:

>> I plan to suppress the warning for ".Fo (f)" just like for ".Fo (*f)"
>> in the future.  The likely typo that should be warned about
>> is ".Fo f()", and that warning will still work.

> Sounds good, thank you!

Committed, see below.

Thanks again for the report!
  Ingo


Log Message:
-----------
Improve validation of function names:
1. Relax checking to accept function types of the form
"ret_type (fname)(args)" (suggested by Yuri Pankov <yuripv dot net>).
2. Tighten checking to require the closing parenthesis.

Modified Files:
--------------
    mandoc:
        mdoc_validate.c
    mandoc/regress/mdoc/Fo:
        warn.in
        warn.out_ascii
        warn.out_lint
        warn.out_markdown

Revision Data
-------------
Index: mdoc_validate.c
===================================================================
RCS file: /home/cvs/mandoc/mandoc/mdoc_validate.c,v
retrieving revision 1.374
retrieving revision 1.375
diff -Lmdoc_validate.c -Lmdoc_validate.c -u -p -r1.374 -r1.375
--- mdoc_validate.c
+++ mdoc_validate.c
@@ -1186,11 +1186,17 @@ post_fname(POST_ARGS)
 	size_t			 pos;
 
 	n = mdoc->last->child;
-	pos = strcspn(n->string, "()");
-	cp = n->string + pos;
-	if ( ! (cp[0] == '\0' || (cp[0] == '(' && cp[1] == '*')))
-		mandoc_msg(MANDOCERR_FN_PAREN, n->line, n->pos + pos,
-		    "%s", n->string);
+	cp = n->string;
+	if (*cp == '(') {
+		if (cp[strlen(cp + 1)] == ')')
+			return;
+		pos = 0;
+	} else {
+		pos = strcspn(cp, "()");
+		if (cp[pos] == '\0')
+			return;
+	}
+	mandoc_msg(MANDOCERR_FN_PAREN, n->line, n->pos + pos, "%s", cp);
 }
 
 static void
Index: warn.in
===================================================================
RCS file: /home/cvs/mandoc/mandoc/regress/mdoc/Fo/warn.in,v
retrieving revision 1.2
retrieving revision 1.3
diff -Lregress/mdoc/Fo/warn.in -Lregress/mdoc/Fo/warn.in -u -p -r1.2 -r1.3
--- regress/mdoc/Fo/warn.in
+++ regress/mdoc/Fo/warn.in
@@ -1,4 +1,4 @@
-.\" $OpenBSD: warn.in,v 1.2 2017/07/04 14:53:25 schwarze Exp $
+.\" $OpenBSD: warn.in,v 1.3 2019/09/13 19:18:48 schwarze Exp $
 .Dd $Mdocdate$
 .Dt FO-WARN 1
 .Os
@@ -12,3 +12,17 @@
 .Fc
 .Ft double
 .Fn atan2 "double y, double x"
+.Ft int
+.Fn close) "int fd"
+.Ft typedef void
+.Fn (handler) int
+.Ft typedef void
+.Fn (*fp) int
+.Ft int
+.Fn (open "const char *path"
+.Ft FILE *
+.Fn (*popen "const char *cmd"
+.Ft void
+.Fn (trail)x void
+.Ft void
+.Fn *star void
Index: warn.out_markdown
===================================================================
RCS file: /home/cvs/mandoc/mandoc/regress/mdoc/Fo/warn.out_markdown,v
retrieving revision 1.2
retrieving revision 1.3
diff -Lregress/mdoc/Fo/warn.out_markdown -Lregress/mdoc/Fo/warn.out_markdown -u -p -r1.2 -r1.3
--- regress/mdoc/Fo/warn.out_markdown
+++ regress/mdoc/Fo/warn.out_markdown
@@ -12,4 +12,25 @@ FO-WARN(1) - General Commands Manual
 *double*  
 **atan2**(*double y, double x*);
 
-OpenBSD - July 4, 2017
+*int*  
+**close)**(*int fd*);
+
+*typedef void*  
+**(handler)**(*int*);
+
+*typedef void*  
+**(\*fp)**(*int*);
+
+*int*  
+**(open**(*const char \*path*);
+
+*FILE \*&zwnj;*  
+**(\*popen**(*const char \*cmd*);
+
+*void*  
+**(trail)x**(*void*);
+
+*void*  
+**\*star**(*void*);
+
+OpenBSD - September 13, 2019
Index: warn.out_lint
===================================================================
RCS file: /home/cvs/mandoc/mandoc/regress/mdoc/Fo/warn.out_lint,v
retrieving revision 1.5
retrieving revision 1.6
diff -Lregress/mdoc/Fo/warn.out_lint -Lregress/mdoc/Fo/warn.out_lint -u -p -r1.5 -r1.6
--- regress/mdoc/Fo/warn.out_lint
+++ regress/mdoc/Fo/warn.out_lint
@@ -1,2 +1,6 @@
 mandoc: warn.in:10:8: WARNING: parenthesis in function name: sin()
 mandoc: warn.in:14:19: WARNING: comma in function argument: double y, double x
+mandoc: warn.in:16:10: WARNING: parenthesis in function name: close)
+mandoc: warn.in:22:5: WARNING: parenthesis in function name: (open
+mandoc: warn.in:24:5: WARNING: parenthesis in function name: (*popen
+mandoc: warn.in:26:5: WARNING: parenthesis in function name: (trail)x
Index: warn.out_ascii
===================================================================
RCS file: /home/cvs/mandoc/mandoc/regress/mdoc/Fo/warn.out_ascii,v
retrieving revision 1.2
retrieving revision 1.3
diff -Lregress/mdoc/Fo/warn.out_ascii -Lregress/mdoc/Fo/warn.out_ascii -u -p -r1.2 -r1.3
--- regress/mdoc/Fo/warn.out_ascii
+++ regress/mdoc/Fo/warn.out_ascii
@@ -10,4 +10,25 @@ S\bSY\bYN\bNO\bOP\bPS\bSI\bIS\bS
      _\bd_\bo_\bu_\bb_\bl_\be
      a\bat\bta\ban\bn2\b2(_\bd_\bo_\bu_\bb_\bl_\be _\by_\b, _\bd_\bo_\bu_\bb_\bl_\be _\bx);
 
-OpenBSD                          July 4, 2017                          OpenBSD
+     _\bi_\bn_\bt
+     c\bcl\blo\bos\bse\be)\b)(_\bi_\bn_\bt _\bf_\bd);
+
+     _\bt_\by_\bp_\be_\bd_\be_\bf _\bv_\bo_\bi_\bd
+     (\b(h\bha\ban\bnd\bdl\ble\ber\br)\b)(_\bi_\bn_\bt);
+
+     _\bt_\by_\bp_\be_\bd_\be_\bf _\bv_\bo_\bi_\bd
+     (\b(*\b*f\bfp\bp)\b)(_\bi_\bn_\bt);
+
+     _\bi_\bn_\bt
+     (\b(o\bop\bpe\ben\bn(_\bc_\bo_\bn_\bs_\bt _\bc_\bh_\ba_\br _\b*_\bp_\ba_\bt_\bh);
+
+     _\bF_\bI_\bL_\bE _\b*
+     (\b(*\b*p\bpo\bop\bpe\ben\bn(_\bc_\bo_\bn_\bs_\bt _\bc_\bh_\ba_\br _\b*_\bc_\bm_\bd);
+
+     _\bv_\bo_\bi_\bd
+     (\b(t\btr\bra\bai\bil\bl)\b)x\bx(_\bv_\bo_\bi_\bd);
+
+     _\bv_\bo_\bi_\bd
+     *\b*s\bst\bta\bar\br(_\bv_\bo_\bi_\bd);
+
+OpenBSD                       September 13, 2019                       OpenBSD
--
 To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv