tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: factoring out prop_XXX_pack_pref() and prop_XXX_copyin_pref()



On Tue, Aug 04, 2009 at 05:30:20PM -0700, Jason Thorpe wrote:
> On Aug 1, 2009, at 5:01 AM, Christoph Badura wrote:
> I prefer the externalize_to and internalize_from names.

Fine with me.  Makes it easier to document.

Is the attached patch agreeable?

I'm not sure what to do about the return value.  The prop*{un,}pack*
forms returned 0 on success and an errno on failure.  The prop*externalize*
functions return a bool indicating success or failure.  So we have a choice
of interface inconsistency:  either make the public prop*externalize_to_pref
functions have a different return value from the *externalize_to_pref
variants (and who thought that failing with giving a reason was a good
interface, anyway?) or make the publick *externalize_to_pref functions
have a different return value from the internal variants.

There are more options.  Personally, I'd favor keeping the interface across
the internal and public functions consistent and treat the *{to,from}_file
functions as the bastards that they are.

> It was, in fact, thought out.  You give it a path name so that it can  
> write to a temporary file in the same directory and then rename to the  
> supplied path, thus ensuring an atomic update of the externalized  
> property list.

That would be a nice utility function in addition to a more general one
that solves the more basic problem of externalizing to and internalizing
from a file descriptor.

Providing a highly specialised interface of limited usefulness instead
of a general one isn't the hallmark of good API design in my book.

But I do not care to continue this part of the thread.

--chris

Index: include/prop/prop_array.h
===================================================================
RCS file: /cvsroot/src/common/include/prop/prop_array.h,v
retrieving revision 1.8
diff -u -r1.8 prop_array.h
--- include/prop/prop_array.h   11 Sep 2008 13:15:13 -0000      1.8
+++ include/prop/prop_array.h   5 Aug 2009 21:55:14 -0000
@@ -1,7 +1,7 @@
 /*     $NetBSD: prop_array.h,v 1.8 2008/09/11 13:15:13 haad Exp $    */
 
 /*-
- * Copyright (c) 2006 The NetBSD Foundation, Inc.
+ * Copyright (c) 2006, 2009 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -66,12 +66,14 @@
 prop_array_t   prop_array_internalize_from_file(const char *);
 
 #if defined(__NetBSD__)
+struct plistref;
+
 #if !defined(_KERNEL) && !defined(_STANDALONE)
+bool           prop_array_externalize_to_pref(prop_array_t, struct plistref *);
 int            prop_array_send_ioctl(prop_array_t, int, unsigned long);
 int            prop_array_recv_ioctl(int, unsigned long, prop_array_t *);
 #elif defined(_KERNEL)
-struct plistref;
-
+int            prop_array_copyin(const struct plistref *, prop_array_t *);
 int            prop_array_copyin_ioctl(const struct plistref *, const u_long,
                                        prop_array_t *);
 int            prop_array_copyout_ioctl(struct plistref *, const u_long,
Index: include/prop/prop_dictionary.h
===================================================================
RCS file: /cvsroot/src/common/include/prop/prop_dictionary.h,v
retrieving revision 1.9
diff -u -r1.9 prop_dictionary.h
--- include/prop/prop_dictionary.h      28 Apr 2008 20:22:51 -0000      1.9
+++ include/prop/prop_dictionary.h      5 Aug 2009 21:55:15 -0000
@@ -1,7 +1,7 @@
 /*     $NetBSD: prop_dictionary.h,v 1.9 2008/04/28 20:22:51 martin Exp $       
*/
 
 /*-
- * Copyright (c) 2006 The NetBSD Foundation, Inc.
+ * Copyright (c) 2006, 2009 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -82,7 +82,10 @@
                                              prop_dictionary_keysym_t);
 
 #if defined(__NetBSD__)
+struct plistref;
+
 #if !defined(_KERNEL) && !defined(_STANDALONE)
+bool           prop_dictionary_externalize_to_pref(prop_dictionary_t, struct 
plistref *);
 int            prop_dictionary_send_ioctl(prop_dictionary_t, int,
                                           unsigned long);
 int            prop_dictionary_recv_ioctl(int, unsigned long,
@@ -91,8 +94,8 @@
                                               int, unsigned long,
                                               prop_dictionary_t *);
 #elif defined(_KERNEL)
-struct plistref;
-
+int            prop_dictionary_copyin(const struct plistref *,
+                                      prop_dictionary_t *);
 int            prop_dictionary_copyin_ioctl(const struct plistref *,
                                             const u_long,
                                             prop_dictionary_t *);
Index: lib/libprop/prop_array.3
===================================================================
RCS file: /cvsroot/src/common/lib/libprop/prop_array.3,v
retrieving revision 1.9
diff -u -r1.9 prop_array.3
--- lib/libprop/prop_array.3    9 Apr 2009 01:18:17 -0000       1.9
+++ lib/libprop/prop_array.3    5 Aug 2009 21:55:15 -0000
@@ -1,6 +1,6 @@
 .\"    $NetBSD: prop_array.3,v 1.9 2009/04/09 01:18:17 joerg Exp $
 .\"
-.\" Copyright (c) 2006 The NetBSD Foundation, Inc.
+.\" Copyright (c) 2006, 2009 The NetBSD Foundation, Inc.
 .\" All rights reserved.
 .\"
 .\" This code is derived from software contributed to The NetBSD Foundation
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd August 19, 2006
+.Dd August 3, 2009
 .Dt PROP_ARRAY 3
 .Os
 .Sh NAME
@@ -50,6 +50,7 @@
 .Nm prop_array_internalize ,
 .Nm prop_array_externalize_to_file ,
 .Nm prop_array_internalize_from_file ,
+.Nm prop_array_externalize_to_pref ,
 .Nm prop_array_equals
 .Nd array property collection object
 .Sh LIBRARY
@@ -102,6 +103,9 @@
 .Fn prop_array_internalize_from_file "const char *path"
 .\"
 .Ft bool
+.Fn prop_array_externalize_to_pref "prop_array_t array" "struct plistref *pref"
+.\"
+.Ft bool
 .Fn prop_array_equals "prop_array_t array1" "prop_array_t array2"
 .Sh DESCRIPTION
 The
@@ -269,6 +273,13 @@
 Returns
 .Dv NULL
 on failure.
+.It Fn prop_array_externalize_to_pref "prop_array_t array" \
+       "struct plistref *pref"
+Externalizes an array and packs it into the plistref specified by
+.Fa pref .
+Returns
+.Dv false
+if externalizing the array fails for any reason.
 .El
 .Sh SEE ALSO
 .Xr prop_bool 3 ,
Index: lib/libprop/prop_copyin_ioctl.9
===================================================================
RCS file: /cvsroot/src/common/lib/libprop/prop_copyin_ioctl.9,v
retrieving revision 1.4
diff -u -r1.4 prop_copyin_ioctl.9
--- lib/libprop/prop_copyin_ioctl.9     30 Apr 2008 13:10:46 -0000      1.4
+++ lib/libprop/prop_copyin_ioctl.9     5 Aug 2009 21:55:15 -0000
@@ -1,6 +1,6 @@
 .\"    $NetBSD: prop_copyin_ioctl.9,v 1.4 2008/04/30 13:10:46 martin Exp $
 .\"
-.\" Copyright (c) 2006 The NetBSD Foundation, Inc.
+.\" Copyright (c) 2006, 2009 The NetBSD Foundation, Inc.
 .\" All rights reserved.
 .\"
 .\" This code is derived from software contributed to The NetBSD Foundation
@@ -27,14 +27,16 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd October 25, 2006
+.Dd August 3, 2009
 .Dt PROP_COPYIN_IOCTL 9
 .Os
 .Sh NAME
 .Nm prop_array_copyin_ioctl ,
 .Nm prop_array_copyout_ioctl ,
+.Nm prop_array_copyin ,
 .Nm prop_dictionary_copyin_ioctl ,
-.Nm prop_dictionary_copyout_ioctl
+.Nm prop_dictionary_copyout_ioctl ,
+.Nm prop_dictionary_copyin
 .Nd Copy property lists to and from kernel space
 .Sh SYNOPSIS
 .In prop/proplib.h
@@ -42,12 +44,18 @@
 .Fn prop_array_copyin_ioctl "const struct plistref *pref" \
     "const u_long cmd" "prop_array_t *arrayp"
 .Ft int
+.Fn prop_array_copyin "const struct plistref *pref" \
+    "prop_array_t *arrayp"
+.Ft int
 .Fn prop_array_copyout_ioctl "struct plistref *pref" \
     "const u_long cmd" "prop_array_t array"
 .Ft int
 .Fn prop_dictionary_copyin_ioctl "const struct plistref *pref" \
     "const u_long cmd" "prop_dictionary_t *dictp"
 .Ft int
+.Fn prop_dictionary_copyin "const struct plistref *pref" \
+    "prop_dictionary_t *dictp"
+.Ft int
 .Fn prop_dictionary_copyout_ioctl "struct plistref *pref" \
     "const u_long cmd" "prop_dictionary_t dict"
 .Sh DESCRIPTION
@@ -60,8 +68,14 @@
 functions implement the kernel side of a protocol for sending property lists
 to and from the kernel using
 .Xr ioctl 2 .
+The functions
+.Nm prop_array_copyin
+and
+.Nm prop_dictionary_copyin
+implement the kernel side of a protocol for sending property lists to the
+kernel as arguments of normal system calls.
 .Pp
-A kernel ioctl routine receiving or returning a property list will be passed a
+A kernel routine receiving or returning a property list will be passed a
 pointer to a
 .Vt struct plistref .
 This structure encapsulates the reference to the property list in externalized
Index: lib/libprop/prop_dictionary.3
===================================================================
RCS file: /cvsroot/src/common/lib/libprop/prop_dictionary.3,v
retrieving revision 1.13
diff -u -r1.13 prop_dictionary.3
--- lib/libprop/prop_dictionary.3       6 May 2008 17:23:38 -0000       1.13
+++ lib/libprop/prop_dictionary.3       5 Aug 2009 21:55:15 -0000
@@ -1,6 +1,6 @@
 .\"    $NetBSD: prop_dictionary.3,v 1.13 2008/05/06 17:23:38 xtraeme Exp $
 .\"
-.\" Copyright (c) 2006 The NetBSD Foundation, Inc.
+.\" Copyright (c) 2006, 2009 The NetBSD Foundation, Inc.
 .\" All rights reserved.
 .\"
 .\" This code is derived from software contributed to The NetBSD Foundation
@@ -27,7 +27,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
 .\" POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd May 6, 2008
+.Dd August 3, 2009
 .Dt PROP_DICTIONARY 3
 .Os
 .Sh NAME
@@ -52,6 +52,8 @@
 .Nm prop_dictionary_internalize ,
 .Nm prop_dictionary_externalize_to_file ,
 .Nm prop_dictionary_internalize_from_file ,
+.Nm prop_dictionary_externalize_to_pref ,
+.Nm prop_dictionary_internalize_from_pref ,
 .Nm prop_dictionary_equals ,
 .Nm prop_dictionary_keysym_cstring_nocopy ,
 .Nm prop_dictionary_keysym_equals
@@ -126,6 +128,10 @@
 .Ft prop_dictionary_t
 .Fn prop_dictionary_internalize_from_file "const char *path"
 .\"
+.Ft bool
+.Fn prop_dictionary_externalize_to_pref "prop_dictionary_t dict" \
+    "struct plistref *pref"
+.\"
 .Sh DESCRIPTION
 The
 .Nm prop_dictionary
@@ -311,6 +317,13 @@
 Returns
 .Dv NULL
 on failure.
+.It Fn prop_dictionary_externalize_to_pref "prop_dictionary_t dict" \
+    "struct plistref *pref"
+Externalizes a dictionary and packs it into the plistref specified by
+.Fa pref .
+Returns
+.Dv false
+if externalizing the dictionary fails for any reason.
 .El
 .Sh SEE ALSO
 .Xr prop_array 3 ,
Index: lib/libprop/prop_kern.c
===================================================================
RCS file: /cvsroot/src/common/lib/libprop/prop_kern.c,v
retrieving revision 1.9
diff -u -r1.9 prop_kern.c
--- lib/libprop/prop_kern.c     28 Apr 2008 20:22:53 -0000      1.9
+++ lib/libprop/prop_kern.c     5 Aug 2009 21:55:15 -0000
@@ -1,7 +1,7 @@
 /*     $NetBSD: prop_kern.c,v 1.9 2008/04/28 20:22:53 martin Exp $     */
 
 /*-
- * Copyright (c) 2006 The NetBSD Foundation, Inc.
+ * Copyright (c) 2006, 2009 The NetBSD Foundation, Inc.
  * All rights reserved.
  *
  * This code is derived from software contributed to The NetBSD Foundation
@@ -44,7 +44,7 @@
 #include <stdio.h>
 
 static int
-_prop_object_pack_pref(prop_object_t obj, struct plistref *pref, char **bufp)
+_prop_object_externalize_to_pref(prop_object_t obj, struct plistref *pref, 
char **bufp)
 {
        char *buf;
 
@@ -70,6 +70,30 @@
        return (0);
 }
 
+/*
+ * prop_array_externalize_to_pref --
+ *     Externalize an array into a plistref for sending to the kernel.
+ */
+bool
+prop_array_externalize_to_pref(prop_array_t array, struct plistref *prefp)
+{
+       char *buf;
+
+       return (_prop_object_externalize_to_pref(array, prefp, &buf) == 0);
+}
+
+/*
+ * prop_dictionary_externalize_to_pref --
+ *     Externalize an dictionary into a plistref for sending to the kernel.
+ */
+bool
+prop_dictionary_externalize_to_pref(prop_dictionary_t dict, struct plistref 
*prefp)
+{
+       char *buf;
+
+       return (_prop_object_externalize_to_pref(dict, prefp, &buf) == 0);
+}
+
 static int
 _prop_object_send_ioctl(prop_object_t obj, int fd, unsigned long cmd)
 {
@@ -77,7 +101,7 @@
        char *buf;
        int error;
 
-       error = _prop_object_pack_pref(obj, &pref, &buf);
+       error = _prop_object_externalize_to_pref(obj, &pref, &buf);
        if (error)
                return (error);
 
@@ -114,7 +138,7 @@
 }
 
 static int
-_prop_object_unpack_pref(const struct plistref *pref, prop_type_t type,
+_prop_object_internalize_from_pref(const struct plistref *pref, prop_type_t 
type,
                         prop_object_t *objp)
 {
        prop_object_t obj = NULL;
@@ -164,7 +188,7 @@
        if (ioctl(fd, cmd, &pref) == -1)
                return (errno);
        
-       return (_prop_object_unpack_pref(&pref, PROP_TYPE_ARRAY,
+       return (_prop_object_internalize_from_pref(&pref, PROP_TYPE_ARRAY,
                                         (prop_object_t *)arrayp));
 }
 
@@ -180,7 +204,7 @@
        if (ioctl(fd, cmd, &pref) == -1)
                return (errno);
 
-       return (_prop_object_unpack_pref(&pref, PROP_TYPE_DICTIONARY,
+       return (_prop_object_internalize_from_pref(&pref, PROP_TYPE_DICTIONARY,
                                         (prop_object_t *)dictp));
 }
 
@@ -197,7 +221,7 @@
        char *buf;
        int error;
 
-       error = _prop_object_pack_pref(dict, &pref, &buf);
+       error = _prop_object_externalize_to_pref(dict, &pref, &buf);
        if (error)
                return (error);
 
@@ -211,7 +235,7 @@
        if (error)
                return (error);
 
-       return (_prop_object_unpack_pref(&pref, PROP_TYPE_DICTIONARY,
+       return (_prop_object_internalize_from_pref(&pref, PROP_TYPE_DICTIONARY,
                            (prop_object_t *)dictp));
 }
 #endif /* !_KERNEL && !_STANDALONE */
@@ -231,16 +255,13 @@
 unsigned int prop_object_copyin_limit = 65536;
 
 static int
-_prop_object_copyin_ioctl(const struct plistref *pref, const prop_type_t type,
-                         const u_long cmd, prop_object_t *objp)
+_prop_object_copyin(const struct plistref *pref, const prop_type_t type,
+                         prop_object_t *objp)
 {
        prop_object_t obj = NULL;
        char *buf;
        int error;
 
-       if ((cmd & IOC_IN) == 0)
-               return (EFAULT);
-
        /*
         * Allocate an extra byte so we can guarantee NUL-termination.
         *
@@ -277,6 +298,40 @@
        return (error);
 }
 
+
+static int
+_prop_object_copyin_ioctl(const struct plistref *pref, const prop_type_t type,
+                         const u_long cmd, prop_object_t *objp)
+{
+       if ((cmd & IOC_IN) == 0)
+               return (EFAULT);
+
+       return _prop_object_copyin(pref, type, objp);
+}
+
+/*
+ * prop_array_copyin --
+ *     Copy in an array passed as a syscall arg.
+ */
+int
+prop_array_copyin(const struct plistref *pref, prop_array_t *arrayp)
+{
+       return (_prop_object_copyin(pref, PROP_TYPE_ARRAY,
+                                         (prop_object_t *)arrayp));
+}
+
+/*
+ * prop_dictionary_copyin --
+ *     Copy in a dictionary passed as a syscall arg.
+ */
+int
+prop_dictionary_copyin(const struct plistref *pref, prop_dictionary_t *dictp)
+{
+       return (_prop_object_copyin(pref, PROP_TYPE_DICTIONARY,
+                                         (prop_object_t *)dictp));
+}
+
+
 /*
  * prop_array_copyin_ioctl --
  *     Copy in an array send with an ioctl.


Home | Main Index | Thread Index | Old Index