On Thu, Nov 29, 2012 at 05:24:12PM +0100, Jilles Tjoelker wrote: > On Thu, Nov 29, 2012 at 02:01:47PM +0200, Konstantin Belousov wrote: > > On Thu, Nov 29, 2012 at 05:16:50AM +0000, Eitan Adler wrote: > > > Author: eadler > > > Date: Thu Nov 29 05:16:50 2012 > > > New Revision: 243665 > > > URL: http://svnweb.freebsd.org/changeset/base/243665 > > > > Log: > > > Mark non-returning function as such > > > > PR: bin/172978 > > > Approved by: cperciva > > > MFC after: 3 days > > > > Modified: > > > head/sbin/dump/dump.h > > > > Modified: head/sbin/dump/dump.h > > > ============================================================================== > > > --- head/sbin/dump/dump.h Thu Nov 29 03:48:39 2012 (r243664) > > > +++ head/sbin/dump/dump.h Thu Nov 29 05:16:50 2012 (r243665) > > > @@ -121,7 +121,7 @@ void trewind(void); > > > void writerec(char *dp, int isspcl); > > > > > > void Exit(int status) __dead2; > > > -void dumpabort(int signo); > > > +void dumpabort(int signo) __dead2; > > > void dump_getfstab(void); > > > > > > char *rawname(char *cp); > > > What is the goal of this change ? > > This has been discussed before in > http://lists.freebsd.org/pipermail/freebsd-current/2011-January/022179.html > and the rest of the thread. The result there seems that it is the least > bad thing to add __dead2 even on static functions that do not return to > reduce compiler/analyzer warnings and improve the generated code with > GCC 4.2.1 and GCC 4.5. But we are already in the shiny new world ? We should not care about so antique and unsupported compilers any more ? Or, at least, the commit message should have said that we do.
I suppose that the worry about slightly unefficient code in the dump(8) would be good April 1 subject. > > > It is arguably backward. There is absolutely no use to mark signal > > handler as __dead2, and all other uses do not benefit from the > > redundand declaration. > > Although most of the uses of dumpabort() are in the same file as its > definition, GCC 4.2.1 is too dumb to detect its noreturn property > automatically. > > Furthermore, some of the uses are in separate files. In particular, > __dead2 on dumpabort() is required to mark or detect quit() as noreturn. > > > Also, being quite removed from the function definition, there is a > > chance that some future modification would make the attribute a lie. > > Both GCC and Clang generate a warning by default if it appears possible > for a noreturn function to return, so as long as -Werror remains enabled > it should be safe enough. The annotation is very much like comment which rephrase the code and thus is useless. But in this case, the attribute annotation does has a chance of changing the compiler behaviour. IMO, such changes shall not be done, and the already accumulated nonsense needs to be reverted.
pgp8DBWb4XHYs.pgp
Description: PGP signature
