Minor change in ufs_quota.c uprintf() fmt

Bruce Evans brde at optusnet.com.au
Fri Jan 14 08:37:06 UTC 2011


On Thu, 13 Jan 2011, Sergey Kandaurov wrote:

> I found a minor issue in sys/ufs/ufs_quota.c: here uprintf()
> suboptimally formats a quota error message with "%s",
> while it is a hardcoded null-terminated character string.

This is mckusick@ style to avoid lines longer than 80 characters without
using unportable C90 string concatenation or (maybe) the style bug of
using C90 string concatenation.  It is common in ffs code.  It is used
a lot even in ffs_softdep.c, which is about 10 years newer than C90.
There it seems to be used mainly for function names.  Those have an
even larger style bug associated with them.  The C99 __func__ is too
often used instead of a literal function name.  This unportability and
style bug is not used in ffs_softdep.c, but the larger unportability
__FUNCTION__ is used in about 1/3 of the panics in compatibility cruft
at the start of ffs_softdep.c (__FUNCTION__ is the 20 year old gcc
spelling of __func__, which was obsoleted by C99 11 years ago except
it is not quite compatible).  Of course, the lines with this style bug
were not written by mckusick@ :-).

> The purpose of my change is to embed the quota message
> into uprintf() fmt itself, thus it will look a bit more correct.
> While here, I fixed whitespaces around there (tab -> 4 spaces).

These style bugs were also not writen by mckusick :-).  Originally, the
%s was needed to split the line according to the style rules because
the statement was more indented so there was no space for the literal
string, and the continuation indent was 4 spaces.  The continuation
indent was broken on 14-Mar-2007 in the same commit that changed the
statement indentation and missed changing the %s.

> I'm going to check it in if nobody objects.  Please, see attached.

% Index: sys/ufs/ufs/ufs_quota.c
% ===================================================================
% --- sys/ufs/ufs/ufs_quota.c	(revision 217308)
% +++ sys/ufs/ufs/ufs_quota.c	(working copy)
% @@ -238,9 +238,9 @@
%  		dq->dq_flags |= DQ_MOD;
%  		DQI_UNLOCK(dq);
%  		if (warn)
% -			uprintf("\n%s: warning, %s %s\n",
% -				ITOV(ip)->v_mount->mnt_stat.f_mntonname,
% -				quotatypes[i], "disk quota exceeded");
% +			uprintf("\n%s: warning, %s disk quota exceeded\n",
% +			    ITOV(ip)->v_mount->mnt_stat.f_mntonname,
% +			    quotatypes[i]);
%  	}
%  	return (0);
%  }

Correct, since the literal now fits.

% @@ -289,10 +289,10 @@
%  			    ip->i_uid == cred->cr_uid) {
%  				dq->dq_flags |= DQ_BLKS;
%  				DQI_UNLOCK(dq);
% -				uprintf("\n%s: write failed, %s %s\n",
% +				uprintf("\n%s: write failed, %s "
% +				    "disk quota exceeded for too long\n",
%  				    ITOV(ip)->v_mount->mnt_stat.f_mntonname,
% -				    quotatypes[type],
% -				    "disk quota exceeded for too long");
% +				    quotatypes[type]);
%  				return (EDQUOT);
%  			}
%  			DQI_UNLOCK(dq);

Just inconsistent with nearby line splitting style.

% @@ -384,9 +384,9 @@
%  		dq->dq_flags |= DQ_MOD;
%  		DQI_UNLOCK(dq);
%  		if (warn)
% -			uprintf("\n%s: warning, %s %s\n",
% -				ITOV(ip)->v_mount->mnt_stat.f_mntonname,
% -				quotatypes[i], "inode quota exceeded");
% +			uprintf("\n%s: warning, %s inode quota exceeded\n",
% +			    ITOV(ip)->v_mount->mnt_stat.f_mntonname,
% +			    quotatypes[i]);
%  	}
%  	return (0);
%  }

Correct.  I didn't check the history of this style bug.

% @@ -434,10 +434,10 @@
%  			    ip->i_uid == cred->cr_uid) {
%  				dq->dq_flags |= DQ_INODS;
%  				DQI_UNLOCK(dq);
% -				uprintf("\n%s: write failed, %s %s\n",
% -					ITOV(ip)->v_mount->mnt_stat.f_mntonname,
% -					quotatypes[type],
% -					"inode quota exceeded for too long");
% +				uprintf("\n%s: write failed, %s "
% +				    "inode quota exceeded for too long\n",
% +				    ITOV(ip)->v_mount->mnt_stat.f_mntonname,
% +				    quotatypes[type]);
%  				return (EDQUOT);
%  			}
%  			DQI_UNLOCK(dq);

Fixes the indentation, but inconsistent with nearby line splitting style.

I don't care about pre-C90 not supporting string concatenation, but don't
like using this concatenation to split strings except at newlines.  It
obfuscates the source code in a similar way to moving literals to an arg
that is decoded by %s, except slightly less :-).  %s is unfortunately
necessary for non-literals.

Bruce


More information about the freebsd-fs mailing list