Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 20 Mar 2018 19:19:01 +1100 (EST)
From:      Bruce Evans <brde@optusnet.com.au>
To:        Ed Maste <emaste@freebsd.org>
Cc:        src-committers@freebsd.org, svn-src-all@freebsd.org,  svn-src-head@freebsd.org
Subject:   Re: svn commit: r331226 - in head/sys: amd64/linux amd64/linux32 i386/linux
Message-ID:  <20180320183744.S950@besplex.bde.org>
In-Reply-To: <201803192126.w2JLQW0N039356@repo.freebsd.org>
References:  <201803192126.w2JLQW0N039356@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 19 Mar 2018, Ed Maste wrote:

> Log:
>  Rename linuxulator functions with linux_ prefix
>
>  It's preferable to have a consistent prefix.  This also reduces
>  differences between the three linux*_sysvec.c files.

It is preferable to have a well-chosen (short...) prefix.  The linux
emulator mostly uses l_ for internal names.  That is a bit too short
for external names.

> Modified: head/sys/amd64/linux/linux_sysvec.c
> ==============================================================================
> --- head/sys/amd64/linux/linux_sysvec.c	Mon Mar 19 21:13:25 2018	(r331225)
> +++ head/sys/amd64/linux/linux_sysvec.c	Mon Mar 19 21:26:32 2018	(r331226)
> @@ -119,14 +119,14 @@ extern struct sysent linux_sysent[LINUX_SYS_MAXSYSCALL
> SET_DECLARE(linux_ioctl_handler_set, struct linux_ioctl_handler);
>
> static register_t * linux_copyout_strings(struct image_params *imgp);
> -static int	elf_linux_fixup(register_t **stack_base,
> +static int	linux_elf_fixup(register_t **stack_base,
> 		    struct image_params *iparams);

'elf_' is an example of a well-chosen prefix.

I don't like the style of putting the verb last in names, but it goes well
with putting prefixes first.  Here the verb placement style is random --
the verb is last in ...handler_set and ...elf_fixup, but not last in
...copyout_strings.

> @@ -180,7 +180,7 @@ LINUX_VDSO_SYM_CHAR(linux_platform);
>  * MPSAFE
>  */
> static int
> -translate_traps(int signal, int trap_code)
> +linux_translate_traps(int signal, int trap_code)

Names without any prefix are likely to have had the verb first, and now
in the middle.

> @@ -245,7 +245,7 @@ linux_set_syscall_retval(struct thread *td, int error)
> }
>
> static int
> -elf_linux_fixup(register_t **stack_base, struct image_params *imgp)
> +linux_fixup_elf(register_t **stack_base, struct image_params *imgp)
> {
> 	Elf_Auxargs *args;
> 	Elf_Addr *base;

The elf_ prefix was first, but is now last (after the verb, unlike
elsewhere for linux_elf_*.

> Modified: head/sys/amd64/linux32/linux32_sysvec.c
> ...
> static void	linux32_fixlimit(struct rlimit *rl, int which);

The random verb order is especially confsing for 'fix' and 'fixup'.
'fixup' is a noun, but is sometimes abused as a verb and adverb instead
of the correct verb and adverb 'fix up'.  When it is placed at the
end, e.g., for limit_fixup(), it is unclear if it is a noun or a verb.
The noun form would mean a function returning a fixup and the verb
form would mean a function fixing up something.  Using 'fix' instead
of fix as a verb at the end is even worse since 'fix' really is both
a noun and a verb.  The normal order in fixlimit makes it clear that
it is used as a verb.

Bruce



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180320183744.S950>