Re: svn commit: r252376 - head/lib/libutil
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
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
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
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
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