Re: [PATCH] contrib/groff Queisce -Wdangling else [updated]

2013-10-27 Thread Sean Bruno 
On Sat, 2013-10-26 at 11:04 -0400, Sean Bruno wrote:
> This adds proper braces to clear Clang warnings about dangling else
> statements in groff.  There is no(intended) functional change.
> 
> http://people.freebsd.org/~sbruno/groff_dangling_else.txt
> 
> sean

I've updated the patch at this link and only put in the needed
parenthesis to quiesce the clang warnings.  Code binary sizes appear
identical after these changes. 

Thanks to Jiles for pointing me at "unintended" code changes.

sean


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] contrib/groff Queisce -Wdangling else

2013-10-27 Thread Sean Bruno 
On Sat, 2013-10-26 at 20:22 -0400, Eitan Adler wrote:
> On Sat, Oct 26, 2013 at 11:04 AM, Sean Bruno  wrote:
> > This adds proper braces to clear Clang warnings about dangling else
> > statements in groff.  There is no(intended) functional change.
> 
> 
> For contributed code why not just disable warnings?  Fixing code is
> good but doing so in our repository instead of upstream doesn't help
> as much.
> 
> 

I believe very strongly that the people who construct compilers know C/C
++ far better than I do, so warnings are their note to me that I'm doing
it wrong.  Disabling warnings is global for a section of the tree (e.g.
groff/roff).  I can't (easily) isolate the warnings individually, so
modifications to the code after I disable the warnings will get excluded
as well, effectively opening the project to crappy code that breaks
things (if the warnings are causing bugs).

For this specific code (groff), it switched to gpl v3 in 2009, so we
won't be doing any more code drops into our tree:

Revision 1.5 - (view) (download) (annotate) - [select for diffs] 
Sun Jan 4 14:50:51 2009 UTC (4 years, 9 months ago) by wl 
Branch: MAIN 
Changes since 1.4: +3 -3 lines 
Diff to previous 1.4 
* */*: Update GPL2 to GPL3.

Therefore, if someone isn't going to rewrite the implementations, its up
to us to maintain the code we have.

sean

"Warnings are meaningful."
FreeBSD Clusteradm/Developer


signature.asc
Description: This is a digitally signed message part

Re: [PATCH] contrib/groff Queisce -Wdangling else

2013-10-26 Thread Eitan Adler 
On Sat, Oct 26, 2013 at 11:04 AM, Sean Bruno  wrote:
> This adds proper braces to clear Clang warnings about dangling else
> statements in groff.  There is no(intended) functional change.


For contributed code why not just disable warnings?  Fixing code is
good but doing so in our repository instead of upstream doesn't help
as much.


-- 
Eitan Adler
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: [PATCH] contrib/groff Queisce -Wdangling else

2013-10-26 Thread Jilles Tjoelker 
On Sat, Oct 26, 2013 at 11:04:29PM +0200, d...@gmx.com wrote:
> Sean Bruno wrote, On 10/26/2013 17:04:

> Index: contrib/groff/src/roff/troff/node.cpp
> ===
> --- contrib/groff/src/roff/troff/node.cpp (revision 257159)
> +++ contrib/groff/src/roff/troff/node.cpp (working copy)
> @@ -4600,17 +4600,18 @@
>}
>else {
>  hunits rem = x - w*i;
> -if (rem > H0)
> +if (rem > H0) {
>if (n->overlaps_horizontally()) {
>   if (out->is_on())
> n->tprint(out);
>   out->right(rem - w);
> +  } else {
> + out->right(rem);
>}
> -  else
> - out->right(rem);
>  while (--i >= 0)
>if (out->is_on())
>   n->tprint(out);
> +}
>}
>  }
> >There is no(intended) functional change.

This part indeed looks wrong. The while loop was not under the if (rem >
H0) but now is. The closing brace should be added before instead of
after the while loop.

Also, putting braces around  out->right(rem);  is not needed.

I recommend making sure the object files do not change due to patches
like these.

-- 
Jilles Tjoelker
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: [PATCH] contrib/groff Queisce -Wdangling else

2013-10-26 Thread Kimmo Paasiala 
On Sun, Oct 27, 2013 at 12:04 AM,   wrote:
> Sean Bruno wrote, On 10/26/2013 17:04:
>

>
> RED ALERT ! SEAN BRUNO IS A GOVERNMENT SPY 1
>

If this is an attempt at humor it's pretty lame one.
___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Re: [PATCH] contrib/groff Queisce -Wdangling else

2013-10-26 Thread dt71 

Sean Bruno wrote, On 10/26/2013 17:04:

Index: contrib/groff/src/roff/troff/node.cpp
===
--- contrib/groff/src/roff/troff/node.cpp   (revision 257159)
+++ contrib/groff/src/roff/troff/node.cpp   (working copy)
@@ -4600,17 +4600,18 @@
   }
   else {
 hunits rem = x - w*i;
-if (rem > H0)
+if (rem > H0) {
   if (n->overlaps_horizontally()) {
if (out->is_on())
  n->tprint(out);
out->right(rem - w);
+  } else {
+   out->right(rem);
   }
-  else
-   out->right(rem);
 while (--i >= 0)
   if (out->is_on())
n->tprint(out);
+}
   }
 }
 

There is no(intended) functional change.


RED ALERT ! SEAN BRUNO IS A GOVERNMENT SPY 1

___
freebsd-current@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

[PATCH] contrib/groff Queisce -Wdangling else

2013-10-26 Thread Sean Bruno 
This adds proper braces to clear Clang warnings about dangling else
statements in groff.  There is no(intended) functional change.

http://people.freebsd.org/~sbruno/groff_dangling_else.txt

sean


signature.asc
Description: This is a digitally signed message part