svn commit: r245222 - head/sys/sys

[Bruce Evans]

On Tue, 15 Jan 2013, Gleb Smirnoff wrote:

> On Thu, Jan 10, 2013 at 09:31:28AM +1100, Bruce Evans wrote:
> B> > On Wed, Jan 09, 2013 at 09:09:09AM +0000, Hans Petter Selasky wrote:
> B> > H> Log:
> B> > H>   Fix compile warning when using GCC:
> B> > H>   Comparison between signed and unsigned.
> B>
> B>         o Add 3 style bugs:
> B>  	 2 sets of excessive parentheses
> B>  	 1 line longer than 80 columns
>
> Bruce,
>
> can you please look at attached patch, that fixes problems you
> describe? It
>
> - fixes mentioned style bugs in param.h
> - adds int casts to MHLEN and MLEN
> - removes extra casts from (void *) to (struct mbuf *)
> - removes extra declarations of inlined functions
> - uninlines and moves m_get2() and m_getjcl() to uipc_mbuf.c
> - size argument of m_get2() swicthed back to int

Looks mostly good.

% ...
% Index: sys/mbuf.h
% ===================================================================
% --- sys/mbuf.h	(revision 245450)
% +++ sys/mbuf.h	(working copy)
% @@ -52,11 +52,14 @@
%   * stored.  Additionally, it is possible to allocate a separate buffer
%   * externally and attach it to the mbuf in a way similar to that of mbuf
%   * clusters.
% + *
% + * MLEN is data length in a normal mbuf.
% + * MHLEN is data length in an mbuf with pktheader.
% + * MINCLSIZE is a smallest amount of data that should be put into cluster.
%   */

The comment needs indent protection if you want to preserve the line
structure of the new comments.  I don't see any better way to format these
lines as bullet points.  A separate paragraph for each would be too verbose.

% -#define	MLEN		(MSIZE - sizeof(struct m_hdr))	/* normal data len */
% -#define	MHLEN		(MLEN - sizeof(struct pkthdr))	/* data len w/pkthdr */
% -#define	MINCLSIZE	(MHLEN + 1)	/* smallest amount to put in cluster */
% -#define	M_MAXCOMPRESS	(MHLEN / 2)	/* max amount to copy for compression */
% +#define	MLEN		((int)(MSIZE - sizeof(struct m_hdr)))
% +#define	MHLEN		((int)(MLEN - sizeof(struct pkthdr)))
% +#define	MINCLSIZE	(MHLEN + 1)

These never needed to be sorted non-alphabetically.

I hope this doesn't cause any problems.  MHLEN, etc., are used just a few
times outside of mbuf.h where we can't control this, mostly in specialized
code.  One interesting use in non-specialized code is in tcp_output():

 		if (len <= MHLEN - hdrlen - max_linkhdr) {

Here len has type long, MHLEN has type size_t (before this change) and type 
int (after this change), hdrlen has type unsigned (misspelled as that instead
of u_int), and max_linkhdr has type int.  So the type combinations are
nonsense, and there is a good chance of getting different compiler warnings
about this before and after the change.  Having just one unsigned type in
the mix tends to promote everything to unsigned, except on 64-bit systems
with one u_int but no size_t, the long has highest rank so everything
gets promoted to long.

% ...
% @@ -393,23 +396,10 @@
%  extern uma_zone_t	zone_jumbo16;
%  extern uma_zone_t	zone_ext_refcnt;
% 
% -static __inline struct mbuf	*m_getcl(int how, short type, int flags);
% ...
% -static __inline void		*m_cljget(struct mbuf *m, int how, int size);
% -static __inline void		 m_chtype(struct mbuf *m, short new_type);
% -void				 mb_free_ext(struct mbuf *);
% -static __inline struct mbuf	*m_last(struct mbuf *m);
% -int				 m_pkthdr_init(struct mbuf *m, int how);
% +struct mbuf	*m_get2(int how, short type, int flags, int size);
% +struct mbuf	*m_getjcl(int how, short type, int flags, int size);
% +void		 mb_free_ext(struct mbuf *);

This one is still missing a parameter name, unlike all (?) the others.

% +int		 m_pkthdr_init(struct mbuf *m, int how);

The 2 new non-inline prototypes should be moved to the general section
for non-inline prototypes.  The 2 old ones must remain early since
they are used in the inlines.

After moving the 2 new ones, also remove their parameter names to match
the (worse) nearby style.  This leaves only 1 nearby other for mb_free_ext()
to be mismatched with.

The general prototype section has a fairly uniform style, with the only
obvious bugs being:
- m_copyup() has parameter names
- m_sanity() is misindented by 1 space
- m_unshare() has 1 parameter name but 2 parameters.

There is another section for non-inline prototypes for packet tag routines.
This has the same style as the general section, except it is unsorted at
the end.  It is placed _before_ the packet tag inlines instead of after
them.  This has the technical advantage that you don't have to split up
the non-inline prototypes.

There is another section for non-inline prototypes and other things for
MBUF_PROFILING.  This has about 3 style bugs per line.

Bruce