Re: svn commit: r252376 - head/lib/libutil

2013-06-30 Thread Dimitry Andric
On Jun 29, 2013, at 18:19, Konstantin Belousov kostik...@gmail.com wrote:
 On Sat, Jun 29, 2013 at 03:52:49PM +, Tim Kientzle wrote:
 Author: kientzle
 Date: Sat Jun 29 15:52:48 2013
 New Revision: 252376
 URL: http://svnweb.freebsd.org/changeset/base/252376
 
 Log:
  Fix -Wunsequenced warning
 What is this ? From the name of the warning, it sounds as if the problem
 is in the lack of sequence point between two modifications of the same
 variable in the expression ?
 
 But, there function' argument evaluation and function call are separated
 by seq point, AFAIR.  Could you, please, clarify ?

Yes, a function call is a sequence point.  The -Wunsequenced warning
was made too aggressive in this trunk upstream commit:
http://llvm.org/viewvc/llvm-project?rev=185035view=rev

I pointed out the problem to the author, and he fixed it in:
http://llvm.org/viewvc/llvm-project?rev=185282view=rev

-Dimitry

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


Re: svn commit: r252376 - head/lib/libutil

2013-06-29 Thread Konstantin Belousov
On Sat, Jun 29, 2013 at 03:52:49PM +, Tim Kientzle wrote:
 Author: kientzle
 Date: Sat Jun 29 15:52:48 2013
 New Revision: 252376
 URL: http://svnweb.freebsd.org/changeset/base/252376
 
 Log:
   Fix -Wunsequenced warning
What is this ? From the name of the warning, it sounds as if the problem
is in the lack of sequence point between two modifications of the same
variable in the expression ?

But, there function' argument evaluation and function call are separated
by seq point, AFAIR.  Could you, please, clarify ?

[Not that I think that the changes are bad]
   
   Submitted by:   d...@gmx.com
 
 Modified:
   head/lib/libutil/login_times.c
 
 Modified: head/lib/libutil/login_times.c
 ==
 --- head/lib/libutil/login_times.cSat Jun 29 15:51:27 2013
 (r252375)
 +++ head/lib/libutil/login_times.cSat Jun 29 15:52:48 2013
 (r252376)
 @@ -96,7 +96,7 @@ parse_lt(const char *str)
   else
   m.lt_start = 0;
   if (*p == '-')
 - p = parse_time(++p, m.lt_end);
 + p = parse_time(p + 1, m.lt_end);
   else
   m.lt_end = 1440;
  



pgpDBijYWt1st.pgp
Description: PGP signature


Re: svn commit: r252376 - head/lib/libutil

2013-06-29 Thread Tim Kientzle

On Jun 29, 2013, at 9:19 AM, Konstantin Belousov wrote:

 On Sat, Jun 29, 2013 at 03:52:49PM +, Tim Kientzle wrote:
 Author: kientzle
 Date: Sat Jun 29 15:52:48 2013
 New Revision: 252376
 URL: http://svnweb.freebsd.org/changeset/base/252376
 
 Log:
  Fix -Wunsequenced warning
 What is this ? From the name of the warning, it sounds as if the problem
 is in the lack of sequence point between two modifications of the same
 variable in the expression ?
 
 But, there function' argument evaluation and function call are separated
 by seq point, AFAIR.  Could you, please, clarify ?

I think you're right about that, though I'd have to
look at the spec to be sure.

Not sure why clang would report this as a -Wunsequenced
warning.  The implied store here is certainly redundant, though.

Tim



  Submitted by:   d...@gmx.com
 
 Modified:
  head/lib/libutil/login_times.c
 
 Modified: head/lib/libutil/login_times.c
 ==
 --- head/lib/libutil/login_times.c   Sat Jun 29 15:51:27 2013
 (r252375)
 +++ head/lib/libutil/login_times.c   Sat Jun 29 15:52:48 2013
 (r252376)
 @@ -96,7 +96,7 @@ parse_lt(const char *str)
  else
  m.lt_start = 0;
  if (*p == '-')
 -p = parse_time(++p, m.lt_end);
 +p = parse_time(p + 1, m.lt_end);
  else
  m.lt_end = 1440;
 
 



signature.asc
Description: Message signed with OpenPGP using GPGMail


Re: svn commit: r252376 - head/lib/libutil

2013-06-29 Thread mdf
On Sat, Jun 29, 2013 at 9:36 AM, Tim Kientzle kient...@freebsd.org wrote:


 On Jun 29, 2013, at 9:19 AM, Konstantin Belousov wrote:

  On Sat, Jun 29, 2013 at 03:52:49PM +, Tim Kientzle wrote:
  Author: kientzle
  Date: Sat Jun 29 15:52:48 2013
  New Revision: 252376
  URL: http://svnweb.freebsd.org/changeset/base/252376
 
  Log:
   Fix -Wunsequenced warning
  What is this ? From the name of the warning, it sounds as if the problem
  is in the lack of sequence point between two modifications of the same
  variable in the expression ?
 
  But, there function' argument evaluation and function call are separated
  by seq point, AFAIR.  Could you, please, clarify ?

 I think you're right about that, though I'd have to
 look at the spec to be sure.

 Not sure why clang would report this as a -Wunsequenced
 warning.  The implied store here is certainly redundant, though.


It may be like other warnings (-Wmissing-field-initializers, I'm looking at
you) that warn about currently correct, but potentially problematic
behavior.

In particular, if any of the functions is re-implemented as a macro, the
sequence point goes away, and this code is broken without the code's author
having made any changes.  So it seems like a reasonable warning.

Thanks,
matthew
___
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to svn-src-all-unsubscr...@freebsd.org


Re: svn commit: r252376 - head/lib/libutil

2013-06-29 Thread Konstantin Belousov
On Sat, Jun 29, 2013 at 09:42:49AM -0700, m...@freebsd.org wrote:
 On Sat, Jun 29, 2013 at 9:36 AM, Tim Kientzle kient...@freebsd.org wrote:
 
 
  On Jun 29, 2013, at 9:19 AM, Konstantin Belousov wrote:
 
   On Sat, Jun 29, 2013 at 03:52:49PM +, Tim Kientzle wrote:
   Author: kientzle
   Date: Sat Jun 29 15:52:48 2013
   New Revision: 252376
   URL: http://svnweb.freebsd.org/changeset/base/252376
  
   Log:
Fix -Wunsequenced warning
   What is this ? From the name of the warning, it sounds as if the problem
   is in the lack of sequence point between two modifications of the same
   variable in the expression ?
  
   But, there function' argument evaluation and function call are separated
   by seq point, AFAIR.  Could you, please, clarify ?
 
  I think you're right about that, though I'd have to
  look at the spec to be sure.
 
  Not sure why clang would report this as a -Wunsequenced
  warning.  The implied store here is certainly redundant, though.
Definitily, I said that the changes are good (not bad).

 
 
 It may be like other warnings (-Wmissing-field-initializers, I'm looking at
 you) that warn about currently correct, but potentially problematic
 behavior.
 
 In particular, if any of the functions is re-implemented as a macro, the
 sequence point goes away, and this code is broken without the code's author
 having made any changes.  So it seems like a reasonable warning.

It is only after the functions reimplemented the code would be broken.
Right now, it seems that the warning is broken, not code.


pgppeVwu5E5lZ.pgp
Description: PGP signature