Date: Sat, 28 Jan 2012 18:39:58 -0800 From: Julian Elischer <julian@freebsd.org> To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, Marius Strobl <marius@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org Subject: Re: svn commit: r230628 - head/sys/sparc64/include Message-ID: <4F24B17E.4050306@freebsd.org> In-Reply-To: <20120128184815.L1114@besplex.bde.org> References: <201201272204.q0RM4hRH062478@svn.freebsd.org> <20120128184815.L1114@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 1/28/12 12:23 AM, Bruce Evans wrote:
> On Fri, 27 Jan 2012, Marius Strobl wrote:
>
>> Log:
>> Mark cpu_{halt,reset}() as __dead2 as appropriate.
>>
>> Modified:
>> head/sys/sparc64/include/cpu.h
>>
>> Modified: head/sys/sparc64/include/cpu.h
>> ==============================================================================
>>
>> --- head/sys/sparc64/include/cpu.h Fri Jan 27 21:52:59 2012
>> (r230627)
>> +++ head/sys/sparc64/include/cpu.h Fri Jan 27 22:04:43 2012
>> (r230628)
>> @@ -53,8 +53,8 @@ extern char btext[];
>> extern char etext[];
>>
>> void cheetah_init(u_int cpu_impl);
>> -void cpu_halt(void);
>> -void cpu_reset(void);
>> +void cpu_halt(void) __dead2;
>> +void cpu_reset(void) __dead2;
>> void fork_trampoline(void);
>> void swi_vm(void *v);
>> void zeus_init(u_int cpu_impl);
>
> This reminds me that these functions and many others shouldn't be
> in <machine/cpu.h> since they necessarily have a MI interface (so
> that MI code can call them). A few interfaces in <machine/cpu.h>
> need to be there so that the can be inlines or macros, but most
> don't. Especially these 2. Since they are extremely non-time-
> critical and not likely to be magical, they can be implemented as
> a wrapper extern function even if the MD code prefers to use an
> in inline or macro. Duplicating these 2 in ${N_ARCH} cpu.h files
> mainly allows some of the files to forget to declare them as
> __dead2.
>
> I put these in <sys/systm.h> and didn't forget __dead2 for them.
> systm.h already has prototypes for 11 functions named cpu_*. These
> are unsorted of course. It also has prototypes for a few functions
> that are misnamed (without a cpu_ as a prefix, or perhaps without
> cpu_ or cpu at all) so that they are naturally unsorted and hard to
> find.
>
> No MD declaration of these 2 in -current except the above has the
> __dead2's. IIRC, I noticed that __dead2 was missing for them from
> the style bug that code after calls to them has /* NOTREACHED */
> comments (shutdown_halt() still does, except in my version).
> __dead2 should have made most of these comments go away 15-20 years
> ago.
>
> A more important "cpu_" function whose comments annoy me is
> cpu_throw(). This has always been declared in an MI file
> (<sys/proc.h>, which shows that we can't expect all these functions
> to be declared in <sys/systm.h> if they are MI). Calls to
> cpu_throw() have the unnecessary comments /* doesn't return */ in
> 2 places (lint won't understand these, so if you are going to
> comment on the obvious then it should be /* NOTREACHED */), and a
> bogus "teapot" panic and a /* NOTREACHED */ comment in another
> place. The panic is so bogus that gcc removes it (as allowed by
> cpu_throw() being declared __dead2). The first 2 places are
> sched_throw() in 2 schedulers. This can't return, but isn't
> declared as __dead2. The other place is thread_exit(). This is
> one of very few extern functions that is declared as __dead2.
>
> Bruce
>
Bruce these comments are valid.
please make use of your commit bit and make the appropriate changes!
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4F24B17E.4050306>