makedoc.c: Warning when -Wall

makedoc.c: Warning when -Wall

[Joel Sherrill]

Hi

I noticed in tbe build output that makedoc.c is compiled without -Wall. I'm
not sure how to add that to the Makefile but when I built it by hand, it
turned up this single warning. I am not sure where the parentheses should
go to address the warning and ensure this is doing what's intended. It
doesn't help that I don't know what the intent is. :).

../newlib-cygwin/newlib/doc/makedoc.c: In function ‘courierize’:
../newlib-cygwin/newlib/doc/makedoc.c:574:6: warning: suggest parentheses
around ‘&&’ within ‘||’ [-Wparentheses]
      && (at(tos, idx+1) == '.')
      ^
Thanks.

--joel

Re: makedoc.c: Warning when -Wall

[Jon Turney]

On 17/06/2021 22:52, Joel Sherrill wrote:
> Hi
> 
> I noticed in tbe build output that makedoc.c is compiled without -Wall. I'm
> not sure how to add that to the Makefile but when I built it by hand, it
> turned up this single warning. I am not sure where the parentheses should
> go to address the warning and ensure this is doing what's intended. It
> doesn't help that I don't know what the intent is. :).
> 
> ../newlib-cygwin/newlib/doc/makedoc.c: In function ‘courierize’:
> ../newlib-cygwin/newlib/doc/makedoc.c:574:6: warning: suggest parentheses
> around ‘&&’ within ‘||’ [-Wparentheses]
>        && (at(tos, idx+1) == '.')
>        ^
> Thanks.

I think the intent is:

	    && (at(tos, idx +1 ) == '.'
		|| at(tos,idx+1) == '|'))

... which is done with correct parenthesization about 30 lines above :)

Re: makedoc.c: Warning when -Wall

[C Howland]

>
>
>
> ------------------------------
> *From:* Newlib <newlib-bounces+craig.howland=caci.com@sourceware.org> on
> behalf of Joel Sherrill <joel@rtems.org>
> *Sent:* Thursday, June 17, 2021 5:52 PM
> *To:* Newlib <newlib@sourceware.org>
> *Subject:* makedoc.c: Warning when -Wall
>
>
> Hi
>
> I noticed in tbe build output that makedoc.c is compiled without -Wall. I'm
> not sure how to add that to the Makefile but when I built it by hand, it
> turned up this single warning. I am not sure where the parentheses should
> go to address the warning and ensure this is doing what's intended. It
> doesn't help that I don't know what the intent is. :).
>
> ../newlib-cygwin/newlib/doc/makedoc.c: In function ‘courierize’:
> ../newlib-cygwin/newlib/doc/makedoc.c:574:6: warning: suggest parentheses
> around ‘&&’ within ‘||’ [-Wparentheses]
>       && (at(tos, idx+1) == '.')
>       ^
> Thanks.
>
The intent is stated (not all that clearly) just before the function, and
the code does not match it.  That is, as coded and as GCC is suggesting
adding is not right.  Here's a suggestion for you that makes the comment
more clear and makes the code match it:
--- makedoc.c 2021-06-17 18:52:19.995015107 -0400
+++ makedoc.c.new 2021-06-17 19:13:31.129643224 -0400
@@ -510,7 +510,7 @@ outputdots (void)

 }

-/* Find lines starting with . and | and put example around them on tos
+/* Find lines starting with either . or | and put example around them on to
    turn
    {*  into open comment and *} into close comment
    escape curlies
@@ -525,19 +525,15 @@ WORD(courierize)

     while (at(tos, idx))
     {
- if (at(tos, idx) == '\n'
-    && (at(tos, idx +1 ) == '.'
- || at(tos,idx+1) == '|'))
+ if (at(tos, idx) == '\n' &&
+    ( (at(tos, idx +1 ) == '.' || at(tos,idx+1) == '|') ) )
  {
     cattext(&out,"\n@smallexample\n");
-    do
-    {
+    do {
  idx += 2;

- while (at(tos, idx) && at(tos, idx)!='\n')
- {
-    if (at(tos,idx)=='{' && at(tos,idx+1) =='*')
-    {
+ while (at(tos, idx) && at(tos, idx)!='\n') {
+    if (at(tos,idx)=='{' && at(tos,idx+1) =='*') {
  cattext(&out," /*");
  idx+=2;
     }
@@ -565,9 +561,8 @@ WORD(courierize)
  }
  catchar(&out,'\n');
     }
-    while (at(tos, idx) == '\n'
-   && (at(tos, idx+1) == '.')
-   || (at(tos,idx+1) == '|'));
+    while (at(tos, idx) == '\n' &&
+   ( (at(tos, idx+1) == '.') || (at(tos,idx+1) == '|') ) );
     cattext(&out,"@end smallexample");
  }
  else

It is rather interesting that the compiler does not warn about the same
comparison made in the if surrounding the do while statement.  (If anyone
reading is good at entering GCC bugs, here's one.)  I fixed both in this
example.
You should confirm that you agree with my clarification of the intent
comment.
The good thing about it is that searching I don't see where this
"courierize" is ever used, so while an apparent bug it is latent for Newlib.
Craig

> --joel
>
>
>