Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 6 Apr 2010 11:33:01 +0200
From:      Gary Jennejohn <gary.jennejohn@freenet.de>
To:        "M. Warner Losh" <imp@bsdimp.com>
Cc:        arch@freebsd.org
Subject:   Re: Minor make cleanup
Message-ID:  <20100406113301.5a535c5c@ernst.jennejohn.org>
In-Reply-To: <20100405.230935.131509470607402730.imp@bsdimp.com>
References:  <20100405.230935.131509470607402730.imp@bsdimp.com>

next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 05 Apr 2010 23:09:35 -0600 (MDT)
"M. Warner Losh" <imp@bsdimp.com> wrote:

> Greetings,
> 
> I'd like to simplify how make gets its MACHINE and MACHINE_ARCH
> settings a little.  Right now we have some special case code for pc98
> that's not been needed for a while (it is to cope on running on
> FreeBSD/pc98 6.x kernels).  Also, the MACHINE and MACHINE_ARCH is a
> little to complex, and can be simplified.  It is now trivial to
> determine machine_arch at run-time, so we do that rather than relying
> on a hard-coded default value.
> 
> I've also removed the MACHINE_CPU setting.  I don't think it is needed
> since we always set it via sys.mk's includsion of bsd.cpu.mk and have
> for a very long time.  It doesn't default to anything sensible anyway.
> 
> Comments?
> 
> Warner
> 
> Index: main.c
> ===================================================================
> --- main.c	(revision 206241)
> +++ main.c	(working copy)
> @@ -872,7 +872,6 @@
>  {
>  	const char *machine;
>  	const char *machine_arch;
> -	const char *machine_cpu;
>  	Boolean outOfDate = TRUE;	/* FALSE if all targets up to date */
>  	const char *p;
>  	const char *pathp;
> @@ -881,6 +880,8 @@
>  	char obpath[MAXPATHLEN];
>  	char cdpath[MAXPATHLEN];
>  	char found_dir[MAXPATHLEN + 1];	/* for searching for sys.mk */
> +	char machine_[40];
> +	char machine_arch_[40];
>  	char *cp = NULL, *start;
>  
>  	save_argv = argv;
> @@ -933,61 +934,30 @@
>  #endif
>  
>  	/*
> -	 * Prior to 7.0, FreeBSD/pc98 kernel used to set the
> -	 * utsname.machine to "i386", and MACHINE was defined as
> -	 * "i386", so it could not be distinguished from FreeBSD/i386.
> -	 * Therefore, we had to check machine.ispc98 and adjust the
> -	 * MACHINE variable.  NOTE: The code is still here to be able
> -	 * to compile new make binary on old FreeBSD/pc98 systems, and
> -	 * have the MACHINE variable set properly.
> +	 * MACHINE is the specific type of machine that is build from
> +	 * MACHINE_ARCH processors.  Often they are the same, but can
> +	 * be different in a number of circumstances.  The kernel for
> +	 * this MACHINE is buils from sys/MACHINE/conf/BLAH.  The cpu
> +	 * support for libc, comes from lib/libc/MACHINE_ARCH/blah.
> +	 *
> +	 * Some ports support muliple disparate types of hardware with
> +	 * a single MACHINE type (eg mips and powerpc support all
> +	 * their differnet kernels under the mips or powerpc umbrella
> +	 * respectively).
>  	 */
>  	if ((machine = getenv("MACHINE")) == NULL) {
> -		int	ispc98;
> -		size_t	len;
> -
> -		len = sizeof(ispc98);
> -		if (!sysctlbyname("machdep.ispc98", &ispc98, &len, NULL, 0)) {
> -			if (ispc98)
> -				machine = "pc98";
> -		}
> +		size_t len = sizeof(machine_);
> +		if (sysctlbyname("hw.machine", machine_, &len, NULL, 0) == 0)
> +			machine = machine_;
>  	}
> -
> -	/*
> -	 * Get the name of this type of MACHINE from utsname
> -	 * so we can share an executable for similar machines.
> -	 * (i.e. m68k: amiga hp300, mac68k, sun3, ...)
> -	 *
> -	 * Note that both MACHINE and MACHINE_ARCH are decided at
> -	 * run-time.
> -	 */
> -	if (machine == NULL) {
> -		static struct utsname utsname;
> -
> -		if (uname(&utsname) == -1)
> -			err(2, "uname");
> -		machine = utsname.machine;
> -	}
> -
>  	if ((machine_arch = getenv("MACHINE_ARCH")) == NULL) {
> -#ifdef MACHINE_ARCH
> -		machine_arch = MACHINE_ARCH;
> -#else
> -		machine_arch = "unknown";
> -#endif
> +		size_t len = sizeof(machine_arch_);
> +		if (sysctlbyname("hw.machine_arch", machine_arch_, &len,
> +		    NULL, 0) == 0)
> +			machine_arch = machine_arch_;
>  	}
>  
>  	/*
> -	 * Set machine_cpu to the minumum supported CPU revision based
> -	 * on the target architecture, if not already set.
> -	 */
> -	if ((machine_cpu = getenv("MACHINE_CPU")) == NULL) {
> -		if (!strcmp(machine_arch, "i386"))
> -			machine_cpu = "i386";
> -		else
> -			machine_cpu = "unknown";
> -	}
> -
> -	/*
>  	 * Initialize the parsing, directory and variable modules to prepare
>  	 * for the reading of inclusion paths and variable settings on the
>  	 * command line
> @@ -1016,7 +986,6 @@
>  	Var_SetGlobal("MFLAGS", "");
>  	Var_SetGlobal("MACHINE", machine);
>  	Var_SetGlobal("MACHINE_ARCH", machine_arch);
> -	Var_SetGlobal("MACHINE_CPU", machine_cpu);
>  #ifdef MAKE_VERSION
>  	Var_SetGlobal("MAKE_VERSION", MAKE_VERSION);
>  #endif
>

I know it's unlikely, but since you're checking whether sysctlbyname()
succeeds or not it seems like you should do what the old code does and
set the variables to "unknown" in case it does fail for completeness.

Lots of typos in the comments :)

--
Gary Jennejohn



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