svn commit: r230037 - head/lib/libutil
Guy Helmer
guy.helmer at palisadesystems.com
Mon Jan 16 17:14:59 UTC 2012
On Jan 14, 2012, at 3:02 PM, Bruce Evans wrote:
> On Sat, 14 Jan 2012, Pawel Jakub Dawidek wrote:
>
>> On Sat, Jan 14, 2012 at 09:59:27PM +1100, Bruce Evans wrote:
>>> ...
>>> It's good to declare mode_t, since pidfile_open() uses it and we want
>>> to remove the dependency on <sys/param.h>. However, this definition
>>> doesn't follow KNF or the style of all the other typedef declarations
>>> in the file, since all the others follow KNF and thus have a space
>>> instead of a tab after #define and also after typedef.
>>
>> I think you mixed space with tab. All the others have a tab after
>> #define and typedef. I fully agree this should be consistent.
>
> Oops.
>
>>>> -#ifdef _SYS_PARAM_H_
>>>> 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
>>>
>>> Now these are unsorted, since a separate section to hold them is not
>>> needed. It was used just to make the ifdef easier to read (we don't
>>> want to split up the main list with blank lines around each ifdef, and
>>> without such blank lines the ifdefs are harder to read).
>>
>> I'd prefer not to change that. All those functions are part of pidfile(3)
>> API and it would be better, IMHO, to keep them together here too.
>
> The functions have a unique prefix, so they are grouped nicely when sorted
> into a long list.
>
> While I'm here, I'll complain about the verboseness of that prefix :-).
> Other APIs in the file mostly use short prefixes:
> - kinfo_. Should have been ki_ like its struct member names. pidfile uses
> a good prefix for its struct member names too.
> - properties_/property_. Bad, like the rest of the API.
> - uu_. A weird nondescriptive name for serial device locking protocol.
> Is it from uucp? But its weirdness makes it memorable, unlike a
> generic English word like `property'. Better yet, I don't have to
> quote it here.
> - f. Stdio's prefix meaning `file'. To fit indentifiers in 8 characters,
> it can't even have an underscore.
> - pw_. Old prefix/abbrieviation for `password'. It's more readable than
> `password' once you are used to it.
> - gr_. Newer prefix for `group'. More verbose than the g in gid.
> - quota_. At least the English word is short.
>
> Just noticed some more disorder: the groups of the defines at the end
> are in random (mostly historical) order (U*, HO*, F*, PW*, HN* (for
> the last parameter of humanize_number()), HN* (for the second last
> parameter...), HD*.
>
> If the pidfile API had defines and if the API is kept in its own
> section, its defines should be in that section. Most of the other APIs
> that have a man page are large enough to deserve the same treatment
> if it is done for pidfile. Some like dehumanize^Wscientificize^W
> humanize_number() are larger although they have fewer functions, since
> they have lots of defines.
>
> Bruce
I've pasted the diff below that I think captures the majority of the issues you have brought up. I have not attempted to tackle the property.3/properties.3 issues, nor the objections to the prefixes that I think would take considerably more effort to resolve -- I wanted to concentrate on the issues that can be isolated to libutil. I hope the diff was pasted OK, especially WRT <tab> characters.
Guy
Index: lib/libutil/property.3
===================================================================
--- lib/libutil/property.3 (revision 230221)
+++ lib/libutil/property.3 (working copy)
@@ -36,7 +36,6 @@
.Sh LIBRARY
.Lb libutil
.Sh SYNOPSIS
-.In sys/types.h
.In libutil.h
.Ft properties
.Fn properties_read "int fd"
Index: lib/libutil/libutil.h
===================================================================
--- lib/libutil/libutil.h (revision 230221)
+++ lib/libutil/libutil.h (working copy)
@@ -49,8 +49,8 @@
#endif
#ifndef _MODE_T_DECLARED
-typedef __mode_t mode_t;
-#define _MODE_T_DECLARED
+typedef __mode_t mode_t;
+#define _MODE_T_DECLARED
#endif
#ifndef _PID_T_DECLARED
@@ -68,8 +68,8 @@
#define _UID_T_DECLARED
#endif
-#define PROPERTY_MAX_NAME 64
-#define PROPERTY_MAX_VALUE 512
+#define PROPERTY_MAX_NAME 64
+#define PROPERTY_MAX_VALUE 512
/* for properties.c */
typedef struct _property {
@@ -80,9 +80,6 @@
/* Avoid pulling in all the include files for no need */
struct in_addr;
-struct kinfo_file;
-struct kinfo_proc;
-struct kinfo_vmentry;
struct pidfh;
struct sockaddr;
struct termios;
@@ -114,6 +111,12 @@
int login_tty(int _fd);
int openpty(int *_amaster, int *_aslave, char *_name,
struct termios *_termp, struct winsize *_winp);
+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);
void properties_free(properties _list);
char *property_find(properties _list, const char *_name);
properties
@@ -170,13 +173,6 @@
int gr_tmp(int _mdf);
#endif
-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);
-
#ifdef _UFS_UFS_QUOTA_H_
struct fstab;
struct quotafile;
@@ -199,22 +195,6 @@
__END_DECLS
-#define UU_LOCK_INUSE (1)
-#define UU_LOCK_OK (0)
-#define UU_LOCK_OPEN_ERR (-1)
-#define UU_LOCK_READ_ERR (-2)
-#define UU_LOCK_CREAT_ERR (-3)
-#define UU_LOCK_WRITE_ERR (-4)
-#define UU_LOCK_LINK_ERR (-5)
-#define UU_LOCK_TRY_ERR (-6)
-#define UU_LOCK_OWNER_ERR (-7)
-
-/* return values from realhostname() */
-#define HOSTNAME_FOUND (0)
-#define HOSTNAME_INCORRECTNAME (1)
-#define HOSTNAME_INVALIDADDR (2)
-#define HOSTNAME_INVALIDNAME (3)
-
/* fparseln(3) */
#define FPARSELN_UNESCESC 0x01
#define FPARSELN_UNESCCONT 0x02
@@ -222,26 +202,43 @@
#define FPARSELN_UNESCREST 0x08
#define FPARSELN_UNESCALL 0x0f
-/* pw_scan() */
-#define PWSCAN_MASTER 0x01
-#define PWSCAN_WARN 0x02
-
-/* humanize_number(3) */
-#define HN_DECIMAL 0x01
-#define HN_NOSPACE 0x02
-#define HN_B 0x04
-#define HN_DIVISOR_1000 0x08
-#define HN_IEC_PREFIXES 0x10
-
-/* maxscale = 0x07 */
-#define HN_GETSCALE 0x10
-#define HN_AUTOSCALE 0x20
-
-/* hexdump(3) */
+/* Flags for hexdump(3) */
#define HD_COLUMN_MASK 0xff
#define HD_DELIM_MASK 0xff00
#define HD_OMIT_COUNT (1 << 16)
#define HD_OMIT_HEX (1 << 17)
#define HD_OMIT_CHARS (1 << 18)
+/* Flags for humanize_number(3) flags */
+#define HN_DECIMAL 0x01
+#define HN_NOSPACE 0x02
+#define HN_B 0x04
+#define HN_DIVISOR_1000 0x08
+#define HN_IEC_PREFIXES 0x10
+
+/* Flags for humanize_number(3) scale */
+#define HN_GETSCALE 0x10
+#define HN_AUTOSCALE 0x20
+
+/* return values from realhostname() */
+#define HOSTNAME_FOUND 0
+#define HOSTNAME_INCORRECTNAME 1
+#define HOSTNAME_INVALIDADDR 2
+#define HOSTNAME_INVALIDNAME 3
+
+/* Flags for pw_scan() */
+#define PWSCAN_MASTER 0x01
+#define PWSCAN_WARN 0x02
+
+/* Return values from uu_lock() */
+#define UU_LOCK_INUSE 1
+#define UU_LOCK_OK 0
+#define UU_LOCK_OPEN_ERR -1
+#define UU_LOCK_READ_ERR -2
+#define UU_LOCK_CREAT_ERR -3
+#define UU_LOCK_WRITE_ERR -4
+#define UU_LOCK_LINK_ERR -5
+#define UU_LOCK_TRY_ERR -6
+#define UU_LOCK_OWNER_ERR -7
+
#endif /* !_LIBUTIL_H_ */
Index: lib/libutil/realhostname.3
===================================================================
--- lib/libutil/realhostname.3 (revision 230221)
+++ lib/libutil/realhostname.3 (working copy)
@@ -33,8 +33,6 @@
.Sh LIBRARY
.Lb libutil
.Sh SYNOPSIS
-.In sys/types.h
-.In netinet/in.h
.In libutil.h
.Ft int
.Fn realhostname "char *host" "size_t hsize" "const struct in_addr *ip"
Index: lib/libutil/pidfile.3
===================================================================
--- lib/libutil/pidfile.3 (revision 230221)
+++ lib/libutil/pidfile.3 (working copy)
@@ -36,7 +36,6 @@
.Sh LIBRARY
.Lb libutil
.Sh SYNOPSIS
-.In sys/param.h
.In libutil.h
.Ft "struct pidfh *"
.Fn pidfile_open "const char *path" "mode_t mode" "pid_t *pidptr"
--------
This message has been scanned by ComplianceSafe, powered by Palisade's PacketSure.
More information about the svn-src-head
mailing list