Arguments format

[Paweł Jasiak]

Thanks for your response!

On 17/04/20, Polytropon wrote:
> On Fri, 17 Apr 2020 18:05:56 +0200, Paweł Jasiak wrote:
> > 1. In sys/mips/mips/autoconf.c we have functions
> > 
> > static void configure_first(dummy)
> > static void configure(dummy)
> > static void configure_final(dummy)
> > 
> > and we are not using argument. We are having those functions also in
> > ricv, arm, arm64, powerpc and x86 and in non of them we are using dummy,
> > so maybe we can just remove it? Or if it is necessary why we don't mark
> > it as __unused like in other functions?
> 
> I haven't checked any further, but I could imagine that
> is has to do with the requirement of those functions
> being able - at least in their declaration - to accept
> a parameter; the type void * is a "somewhat universal"
> type. Note that the functions are being mentioned in
> macros, such as
> 
> 	SYSINIT(configure1, SI_SUB_CONFIGURE, SI_ORDER_FIRST, configure_first, NULL);
> 
> in /usr/src/sys/powerpc/powerpc/autoconf.c which might
> be the reason why there has to be a dummy parameter...
> 
> Okay, further investigation. ;-)
> 
> According to "man 9 SYSINIT", the definition is
> 
>      SYSINIT(uniquifier, enum sysinit_sub_id subsystem,
>          enum sysinit_elem_order order, sysinit_cfunc_t func,
>          const void *ident);
> 
> and the type sysinit_cfunc_t is defined as
> 
> 	typedef void (*sysinit_cfunc_t)(const void *);
> 
> in /usr/src/sys/sys/kernel.h, so this is the reaon why
> the configure_first(), configure(), and configure_final()
> functions have to be "compatible".

Thanks, I didn't really pay attention to SYSINIT but still don't 
understand why we don't mark the arguments as __unused.

> > 2. Above functions have strange definition for arguments.
> > 
> >     static void
> >     configure(dummy)
> >         void *dummy;
> >     {
> >         ...
> >     }
> > 
> > Why we are not using
> > 
> >     static void
> >     configure(void *dummy)
> >     {
> >         ...
> >     }
> > 
> > like in other places?
> 
> That is not a strange format, it's an older dialect of C,
> usually called "K&R C", where the definition of a function
> typically is:
> 
> 	return-type function-name(arg1, arg2, arg3, ...)
> 		type arg1;
> 		type arg2;
> 		type arg3;
> 		...
> 	{
> 		function-body
> 	}
> 
> A convention also is to put the function's return type on
> an individual line, so the function's name always starts
> at column 1.
> 
> See "man 9 style" for details.
> 
> Still, this style is not being followed consistently:
> 
> % grep "^configure_first" `find /usr/src/sys -name autoconf.c`
> /usr/src/sys/x86/x86/autoconf.c:configure_first(void *dummy)
> /usr/src/sys/arm64/arm64/autoconf.c:configure_first(void *dummy)
> /usr/src/sys/arm/arm/autoconf.c:configure_first(void *dummy)
> /usr/src/sys/riscv/riscv/autoconf.c:configure_first(void *dummy)
> /usr/src/sys/sparc64/sparc64/autoconf.c:configure_first(void *dummy)
> /usr/src/sys/powerpc/powerpc/autoconf.c:configure_first(void *dummy)
> /usr/src/sys/mips/mips/autoconf.c:configure_first(dummy)
> 
> Some use "K&R C" style, others use "ANSI C" style.

Thanks, I know both styles and I was worried about mixing them.

> > 3. In sys/mips/mips/octeon_cop2.c we are having
> > 
> >     struct octeon_cop2_state *
> >     octeon_cop2_alloc_ctx()
> >     {
> >         ...
> >     }
> > but it's declaration in sys/mips/include/octeon_cop2.h is
> > 
> >     struct octeon_cop2_state* octeon_cop2_alloc_ctx(void);
> > 
> > Question is if we should change octeon_cop2_alloc_ctx() into
> > octeon_cop2_alloc_ctx(void)?
> 
> There is a difference between () and (void) which _might_ be
> intended; however, prototype and declaration should in fact
> have the same signature. If the argument is (), the function
> will accept any parameters, including none ("any parameters
> list"); if it's (void), the function will refuse to accept
> any parameters ("emtpy parameter list"), which is explicit
> for "it doesn't use any parameters".

I know the difference again ;) 

% grep -nr "octeon_cop2_alloc_ctx"
sys/mips/mips/vm_machdep.c:164:             td2->td_md.md_cop2 = octeon_cop2_alloc_ctx();
sys/mips/mips/vm_machdep.c:169:             td2->td_md.md_ucop2 = octeon_cop2_alloc_ctx();
sys/mips/mips/octeon_cop2.c:53:octeon_cop2_alloc_ctx()
sys/mips/mips/trap.c:942:                   td->td_md.md_cop2 = octeon_cop2_alloc_ctx();
sys/mips/mips/trap.c:995:                           td->td_md.md_ucop2 = octeon_cop2_alloc_ctx();
sys/mips/include/octeon_cop2.h:208:struct octeon_cop2_state* octeon_cop2_alloc_ctx(void);


I believe that all uses of the octeon_cop2_alloc_ctx function so I still
don't understand why we have different signatures.

Thanks!

-- 

Paweł Jasiak
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-questions/attachments/20200418/a3a49242/attachment.sig>