From owner-freebsd-arch@FreeBSD.ORG Thu Jun 2 02:06:22 2005 Return-Path: X-Original-To: freebsd-arch@freebsd.org Delivered-To: freebsd-arch@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 1466916A41C; Thu, 2 Jun 2005 02:06:22 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailout1.pacific.net.au (mailout1.pacific.net.au [61.8.0.84]) by mx1.FreeBSD.org (Postfix) with ESMTP id 8F7C643D4C; Thu, 2 Jun 2005 02:06:21 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy2.pacific.net.au (mailproxy2.pacific.net.au [61.8.0.87]) by mailout1.pacific.net.au (8.12.3/8.12.3/Debian-7.1) with ESMTP id j52268rI011333; Thu, 2 Jun 2005 12:06:08 +1000 Received: from katana.zip.com.au (katana.zip.com.au [61.8.7.246]) by mailproxy2.pacific.net.au (8.12.3/8.12.3/Debian-7.1) with ESMTP id j52265MC003007; Thu, 2 Jun 2005 12:06:06 +1000 Date: Thu, 2 Jun 2005 12:06:07 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Harti Brandt In-Reply-To: <20050601170858.V51549@beagle.kn.op.dlr.de> Message-ID: <20050602114859.C98072@delplex.bde.org> References: <1117613456.771.16.camel@spirit> <20050601150344.GA39784@dragon.NUXI.org> <20050601170858.V51549@beagle.kn.op.dlr.de> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: delphij@delphij.net, freebsd-arch@freebsd.org Subject: Re: [PATCH RFC] Add a macro for null mount options to sbin/mount* X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 02 Jun 2005 02:06:22 -0000 On Wed, 1 Jun 2005, Harti Brandt wrote: > On Wed, 1 Jun 2005, David O'Brien wrote: > > DO>On Wed, Jun 01, 2005 at 04:10:56PM +0800, Xin LI wrote: > DO>> Hi, -arch@, > DO>> > DO>> In our mount* utilities, the null mount option, which is usually be used > DO>> as a terminator of an option vector, is defined with some hand-rolled > DO>> terms, e.g.: {NULL}, {NULL, 0, 0, 0}, etc. > DO>> > DO>> I think it would be nice to have a new macro to deal with this, say, > DO>> MOPT_NULL, which would be extended to {NULL, 0, 0, 0}, which can act as > DO>> an explicit initialize. And in my opinion, something like: > DO> > DO>I think it is better to leave it alone. The "NULL" termination of a list > DO>like this is a C idiom that should be clear to any C programmer. Hiding > DO>the details in a macro (is MOPT_NULL an integer or a sentinel?) makes it > DO>harder to see the idiom and know exactly what is going on and how this > DO>list will be processed. I tend to agree. Here is the code that uses the terminator. From getmntopts.c: % /* Scan option table. */ % for (m = m0; m->m_option != NULL; ++m) { ^^^^^^^^^^^^^^^^^^^^ (1) % len = strlen(m->m_option); % if (strncasecmp(opt, m->m_option, len) == 0) % if ( m->m_option[len] == '\0' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (2) % || m->m_option[len] == '=' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (2) % ) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ (2) % break; % } % % /* Save flag, or fail if option is not recognized. */ % if (m->m_option) { ^^^^^^^^^^^ (2) (1) Correct use of terminator. (2) Large logic and formatting style bugs were added in rev.1.2. (3) Obfuscated use of the terminator (no != NULL, unlike in the previous expression of the same condition). > The problem is that with the right set of warning options gcc will warn if > you write {NULL}, but there are more fields than just a pointer. If the > structure definition is stable enough this is no problem - just go through > all the programs and fix it once. If the definition is likely to change > from time to time, a macro is better because it just does the right thing. It would be a compiler bug to complain about missing initializaters for trailing fields -- consider explicitly initalizing all the data in "struct foo { char *p; int n[1024 * 1024]; }". Here the field holding the NULL is the only one that is used. The definition of the terminator would only need to change if this field is moved. Bruce