From owner-cvs-src-old@FreeBSD.ORG Sun Dec 7 19:42:23 2008 Return-Path: Delivered-To: cvs-src-old@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 22701106564A for ; Sun, 7 Dec 2008 19:42:23 +0000 (UTC) (envelope-from luigi@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 0F1138FC08 for ; Sun, 7 Dec 2008 19:42:23 +0000 (UTC) (envelope-from luigi@FreeBSD.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id mB7JgMfS012175 for ; Sun, 7 Dec 2008 19:42:22 GMT (envelope-from luigi@repoman.freebsd.org) Received: (from svn2cvs@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id mB7JgMUd012174 for cvs-src-old@freebsd.org; Sun, 7 Dec 2008 19:42:22 GMT (envelope-from luigi@repoman.freebsd.org) Message-Id: <200812071942.mB7JgMUd012174@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: svn2cvs set sender to luigi@repoman.freebsd.org using -f From: Luigi Rizzo Date: Sun, 7 Dec 2008 19:42:20 +0000 (UTC) To: cvs-src-old@freebsd.org X-FreeBSD-CVS-Branch: HEAD Subject: cvs commit: src/sys/boot/forth support.4th X-BeenThere: cvs-src-old@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: **OBSOLETE** CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 07 Dec 2008 19:42:23 -0000 luigi 2008-12-07 19:42:20 UTC FreeBSD src repository Modified files: sys/boot/forth support.4th Log: SVN rev 185746 on 2008-12-07 19:42:20Z by luigi PROBLEM: putting in a loader config file a line of the form loader_conf_files="foo bar baz" should cause loading the files listed, and then resume with the remaining config files (from previous values of the variable). Unfortunately, sometimes the line was ignored -- actually even modifying the line in /boot/default/loader.conf sometimes doesn't work. ANALYSIS: After much investigation, turned out to be a bug in the logic. The existing code detected a new assignment by looking at the address of the the variable containing the string. This only worked by pure chance, i.e. if the new string is longer than the previous value then the memory allocator may return a different address to store the string hence triggering the detection. SOLUTION: This commit contains a minimal change to fix the problem, without altering too much the existing structure of the code. However, as a step towards improving the quality and reliability of this code, I have introduced a handful of one-line functions (strget, strset, strfree, string= ) that could be used in dozens of places in the existing code. HOWEVER: There is a much bigger problem here. Even though I am no Forth expert (as most fellow src committers) I can tell that much of the forth code (in support.4th at least) is in severe need of a review/refactoring: + pieces of code are replicated multiple times instead of writing functions (see e.g. set_module_*); + a lot of stale code (e.g. "structure" definitions for preloaded_files, kernel_module, pnp stuff) which is not used or at least belongs elsewhere. The code bload is extremely bad as the loader runs with very small memory constraints, and we already hit the limit once (see http://svn.freebsd.org/viewvc/base?view=revision&revision=185132 Reducing the footprint of the forth files is critical. + two different styles of coding, one using pure stack functions (maybe beautiful but surely highly unreadable), one using high level mechanisms to give names to arguments and local variables (which leads to readable code). Note that this code is used by default by all FreeBSD installations, so the fragility and the code bloat are extremely damaging. I will try to work fixing the three items above, but if others have time, please have a look at these issues. MFC after: 4 weeks Revision Changes Path 1.18 +19 -26 src/sys/boot/forth/support.4th