svn commit: r229937 - in head/lib: libc/gen libutil

Guy Helmer guy.helmer at palisadesystems.com
Wed Jan 11 15:10:19 UTC 2012


On Jan 11, 2012, at 6:25 AM, Bruce Evans wrote:

> On Tue, 10 Jan 2012, Pawel Jakub Dawidek wrote:
> 
>> On Tue, Jan 10, 2012 at 07:53:25PM +0000, Guy Helmer wrote:
>> [...]
>>> Modified: head/lib/libutil/libutil.h
>>> ==============================================================================
>>> --- head/lib/libutil/libutil.h	Tue Jan 10 18:43:27 2012	(r229936)
>>> +++ head/lib/libutil/libutil.h	Tue Jan 10 19:53:25 2012	(r229937)
>>> @@ -170,6 +170,7 @@ struct pidfh *pidfile_open(const char *p
>>> int pidfile_write(struct pidfh *pfh);
>>> int pidfile_close(struct pidfh *pfh);
>>> int pidfile_remove(struct pidfh *pfh);
>>> +int pidfile_fileno(struct pidfh *pfh);
>>> #endif
>>> 
>>> #ifdef _UFS_UFS_QUOTA_H_
>> 
>> One more thing. You also need to add link in Makefile, so that
>> 'man pidfile_fileno' will work.
> 
> Is there any chance of keeping sorted lists sorted?
> 
> This file used to be mostly sorted and mostly in KNF (tab before
> function names) and mostly without namespace pollution in parameter
> names).  Newer code in it violates all of these style and header
> implementation rules.  The pidfile code was already especially bad,
> and adding to the end of the unsorted list in it doesn't help.
> 
> The pidfile man page is also unsorted.  It was in "operations" order
> "open/write/close/remove".  That is hard to maintain and hard to search
> for long lists.  Adding pidfile_fileno() to the end of the list in the
> same disorder as above makes the list not even in "operations" order
> (since pidfile_fileno() is only valid betwen pidfile_open() and
> pidfile_close()).
> 
> Old disorder and nearby bugs in libutil.h (old := before 2005):
> - the forward declarations of structs are totally disordered
> - the following prototypes are disordered (mostly by adding them to
>  the end of a orginally-almost-sorted list:
>    trimdomain(), forkpty(), humanize_number(), uu_lockerr(), uu_unlock(),
>    _secure_path(), properties_read(), properties_free(), properties_find(),
>    auth_getval(), realhostname(),
> - the forward declaration of struct sockaddr is disorderd (not unsorted
>  at the beginning with the others)
> - surprisingly, realhostname_sa() is not unsorted relative to realhostname()
> - the following prototypes have namespace pollution in parameter names
>  (their parameter names are in the implementation namespace, unlike for
>  all of the older prototypes in the file):
>    properties read(), properties free(), properties free(), auth_getval(),
>    realhostname(), realhostname_sa().  The properties code used to be the
>    only really ugly code in this file.
> - formatting errors for openpty() (premature line splitting and wrong
>  continuation indent).  The very first prototype in this file gives
>  an example of normal splitting and continuation indent
> - formatting errors for forkpty() (same as for openpty())
> - line too long for realhostname_sa(), pw_copy()
> - consider fparseln() as being in a new section so it isn't unsorted.
>  The start of this section should be delimited by a blank line (this is
>  done in -current).  The new section isn't really justified.  It is
>  just 1 function ifdefed to avoid a namespace problem.
> - no namespace pollution for fparseln()'s parameters, but that it because
>  it is in a style different from all older prototypes in the file -- it
>  doesn't name its parameters.
> 
> Newer disorder and nearby bugs:
> - further unsorting of the forward declarations by adding kinfo* to the
>  end of the unsorted list.  At least the new declarations are sorted
>  internally.
> - the following protypes in the early sections are disordered:
>    expand_number(), kld*(), kinfo* (no need for a new section for the
>    last 2.  At least they are each sorted internally), gr_scan()
> - the following prototypes have namespace pollution in parameter names:
>    hexdump(), kld*(), gr_dup(), gr_equal(), gr_make(), gr_scan().  Only
>    about half of the gr*() prototypes have this bug
> - better indentation for kinfo*() than for older or newer entries!
> - line too long for gr_entry()
> - pidfile*() have all of the above bugs, plus they are the only prototypes
>  up to this point in the file that don't put a tab before the function
>  name when the return type is int
> - quota*() are slightly worse than pidfile*().  They add the additional
>  style bug of not using parameter names at all, except for 1 of 2
>  parameters in 1 of 12 prototypes.  That parameter is namespace pollution.
> 
> Bruce

Does this patch improve the situation?

I have not found guidance for line lengths in style(9) -- am I looking in the wrong place?

Guy

Index: libutil.h
===================================================================
--- libutil.h	(revision 229961)
+++ libutil.h	(working copy)
@@ -84,110 +84,117 @@
 #endif
 
 /* Avoid pulling in all the include files for no need */
-struct termios;
-struct winsize;
 struct in_addr;
 struct kinfo_file;
 struct kinfo_proc;
 struct kinfo_vmentry;
+struct sockaddr;
+struct termios;
+struct winsize;
 
 __BEGIN_DECLS
-void	clean_environment(const char * const *_white,
+char	*auth_getval(const char *_name);
+void	 clean_environment(const char * const *_white,
 	    const char * const *_more_white);
-int	extattr_namespace_to_string(int _attrnamespace, char **_string);
-int	extattr_string_to_namespace(const char *_string, int *_attrnamespace);
-int	flopen(const char *_path, int _flags, ...);
-void	hexdump(const void *ptr, int length, const char *hdr, int flags);
-int	login_tty(int _fd);
-void	trimdomain(char *_fullhost, int _hostsize);
-int	openpty(int *_amaster, int *_aslave, char *_name,
-		     struct termios *_termp, struct winsize *_winp);
-int	forkpty(int *_amaster, char *_name,
-		     struct termios *_termp, struct winsize *_winp);
-int	humanize_number(char *_buf, size_t _len, int64_t _number,
+int	 expand_number(const char *_buf, uint64_t *_num);
+int	 extattr_namespace_to_string(int _attrnamespace, char **_string);
+int	 extattr_string_to_namespace(const char *_string, int *_attrnamespace);
+int	 flopen(const char *_path, int _flags, ...);
+int	 forkpty(int *_amaster, char *_name,
+	    struct termios *_termp, struct winsize *_winp);
+void	 hexdump(const void *_ptr, int _length, const char *_hdr, int _flags);
+int	 humanize_number(char *_buf, size_t _len, int64_t _number,
 	    const char *_suffix, int _scale, int _flags);
-int	expand_number(const char *_buf, uint64_t *_num);
-const char *uu_lockerr(int _uu_lockresult);
-int	uu_lock(const char *_ttyname);
-int	uu_unlock(const char *_ttyname);
-int	uu_lock_txfr(const char *_ttyname, pid_t _pid);
-int	_secure_path(const char *_path, uid_t _uid, gid_t _gid);
-properties properties_read(int fd);
-void	properties_free(properties list);
-char	*property_find(properties list, const char *name);
-char	*auth_getval(const char *name);
-int	realhostname(char *host, size_t hsize, const struct in_addr *ip);
-struct sockaddr;
-int	realhostname_sa(char *host, size_t hsize, struct sockaddr *addr,
-			     int addrlen);
-
-int	kld_isloaded(const char *name);
-int	kld_load(const char *name);
 struct kinfo_file *
-	kinfo_getfile(pid_t _pid, int *_cntp);
+	 kinfo_getfile(pid_t _pid, int *_cntp);
 struct kinfo_vmentry *
-	kinfo_getvmmap(pid_t _pid, int *_cntp);
+	 kinfo_getvmmap(pid_t _pid, int *_cntp);
 struct kinfo_proc *
-	kinfo_getallproc(int *_cntp);
+	 kinfo_getallproc(int *_cntp);
 struct kinfo_proc *
-	kinfo_getproc(pid_t _pid);
+	 kinfo_getproc(pid_t _pid);
+int	 kld_isloaded(const char *_name);
+int	 kld_load(const char *_name);
+int	 login_tty(int _fd);
+int	 openpty(int *_amaster, int *_aslave, char *_name,
+	    struct termios *_termp, struct winsize *_winp);
+void	 properties_free(properties _list);
+properties
+	 properties_read(int fd);
+char	*property_find(properties _list, const char *_name);
+int	 realhostname(char *_host, size_t _hsize, const struct in_addr *_ip);
+int	 realhostname_sa(char *_host, size_t _hsize, struct sockaddr *_addr,
+	    int _addrlen);
+void	 trimdomain(char *_fullhost, int _hostsize);
+const char *
+	 uu_lockerr(int _uu_lockresult);
+int	 uu_lock(const char *_ttyname);
+int	 uu_unlock(const char *_ttyname);
+int	 uu_lock_txfr(const char *_ttyname, pid_t _pid);
+int	 _secure_path(const char *_path, uid_t _uid, gid_t _gid);
 
+
 #ifdef _STDIO_H_	/* avoid adding new includes */
-char   *fparseln(FILE *, size_t *, size_t *, const char[3], int);
+char   *fparseln(FILE *_fp, size_t *_len, size_t *_lineno,
+	    const char _delim[3], int _flags);
 #endif
 
 #ifdef _PWD_H_
-int	pw_copy(int _ffd, int _tfd, const struct passwd *_pw, struct passwd *_old_pw);
+int	 pw_copy(int _ffd, int _tfd, const struct passwd *_pw, struct passwd *_old_pw);
 struct passwd *pw_dup(const struct passwd *_pw);
-int	pw_edit(int _notsetuid);
-int	pw_equal(const struct passwd *_pw1, const struct passwd *_pw2);
-void	pw_fini(void);
-int	pw_init(const char *_dir, const char *_master);
+int	 pw_edit(int _notsetuid);
+int	 pw_equal(const struct passwd *_pw1, const struct passwd *_pw2);
+void	 pw_fini(void);
+int	 pw_init(const char *_dir, const char *_master);
 char	*pw_make(const struct passwd *_pw);
 char	*pw_make_v7(const struct passwd *_pw);
-int	pw_mkdb(const char *_user);
-int	pw_lock(void);
+int	 pw_mkdb(const char *_user);
+int	 pw_lock(void);
 struct passwd *pw_scan(const char *_line, int _flags);
 const char *pw_tempname(void);
-int	pw_tmp(int _mfd);
+int	 pw_tmp(int _mfd);
 #endif
 
 #ifdef _GRP_H_
-int 	gr_copy(int __ffd, int _tfd, const struct group *_gr, struct group *_old_gr);
-struct group *gr_dup(const struct group *gr);
-int	gr_equal(const struct group *gr1, const struct group *gr2);
-void	gr_fini(void);
-int	gr_init(const char *_dir, const char *_master);
-int	gr_lock(void);
-char	*gr_make(const struct group *gr);
-int	gr_mkdb(void);
-int	gr_tmp(int _mdf);
-struct group *gr_scan(const char *line);
+int 	 gr_copy(int __ffd, int _tfd, const struct group *_gr, struct group *_old_gr);
+struct group *gr_dup(const struct group *_gr);
+int	 gr_equal(const struct group *_gr1, const struct group *_gr2);
+void	 gr_fini(void);
+int	 gr_init(const char *_dir, const char *_master);
+int	 gr_lock(void);
+char	*gr_make(const struct group *_gr);
+int	 gr_mkdb(void);
+struct group *gr_scan(const char *_line);
+int	 gr_tmp(int _mdf);
 #endif
 
 #ifdef _SYS_PARAM_H_
-struct pidfh *pidfile_open(const char *path, mode_t mode, pid_t *pidptr);
-int pidfile_write(struct pidfh *pfh);
-int pidfile_close(struct pidfh *pfh);
-int pidfile_remove(struct pidfh *pfh);
-int pidfile_fileno(const struct pidfh *pfh);
+int	 pidfile_close(struct pidfh *_pfh);
+int	 pidfile_fileno(const struct pidfh *_pfh);
+struct pidfh *
+	 pidfile_open(const char *_path, mode_t _mode, pid_t *_pidptr);
+int	 pidfile_remove(struct pidfh *_pfh);
+int	 pidfile_write(struct pidfh *_pfh);
 #endif
 
 #ifdef _UFS_UFS_QUOTA_H_
 struct quotafile;
 struct fstab;
-struct quotafile *quota_open(struct fstab *, int, int);
-void quota_close(struct quotafile *);
-int quota_on(struct quotafile *);
-int quota_off(struct quotafile *);
-const char *quota_fsname(const struct quotafile *);
-const char *quota_qfname(const struct quotafile *);
-int quota_maxid(struct quotafile *);
-int quota_check_path(const struct quotafile *, const char *path);
-int quota_read(struct quotafile *, struct dqblk *, int);
-int quota_write_limits(struct quotafile *, struct dqblk *, int);
-int quota_write_usage(struct quotafile *, struct dqblk *, int);
-int quota_convert(struct quotafile *, int);
+int	 quota_check_path(const struct quotafile *_qf, const char *_path);
+void	 quota_close(struct quotafile *_qf);
+int	 quota_convert(struct quotafile *_qf, int _wordsize);
+const char *
+	 quota_fsname(const struct quotafile *_qf);
+int	 quota_maxid(struct quotafile *_qf);
+int	 quota_off(struct quotafile *_qf);
+int	 quota_on(struct quotafile *_qf);
+struct quotafile *
+	 quota_open(struct fstab *_fs, int _quotatype, int _openflags);
+const char *
+	 quota_qfname(const struct quotafile *_qf);
+int	 quota_read(struct quotafile *_qf, struct dqblk *_dqb, int _id);
+int	 quota_write_limits(struct quotafile *_qf, struct dqblk *_dqb, int _id);
+int	 quota_write_usage(struct quotafile *_qf, struct dqblk *_dqb, int _id);
 #endif
 
 __END_DECLS

--------
This message has been scanned by ComplianceSafe, powered by Palisade's PacketSure.


More information about the svn-src-head mailing list