[PATCH RFC] Add a macro for null mount options to sbin/mount*

[Bruce Evans]

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