Sync up python stuff from HEAD.

(This used to be commit 92c76e16fa40adc6148b9c11b3e377f35c464975)

7 files changed, 463 insertions, 303 deletions

diff --git a/source3/python/examples/tdbpack/test_tdbpack.py b/source3/python/examples/tdbpack/test_tdbpack.py
index 659dc0efed..83282e745e 100755
--- a/source3/python/examples/tdbpack/test_tdbpack.py
+++ b/source3/python/examples/tdbpack/test_tdbpack.py
@@ -22,9 +22,40 @@ import samba.tdbpack
 
 both_unpackers = (samba.tdbpack.unpack, oldtdbutil.unpack)
 both_packers = (samba.tdbpack.pack, oldtdbutil.pack)
+
+
+
+# #             ('B', [10, 'hello'], '\x0a\0\0\0hello'),
+#              ('BB', [11, 'hello\0world', 3, 'now'],
+#               '\x0b\0\0\0hello\0world\x03\0\0\0now'),
+#              ('pd', [1, 10], '\x01\0\0\0\x0a\0\0\0'),
+#              ('BBB', [5, 'hello', 0, '', 5, 'world'],
+#               '\x05\0\0\0hello\0\0\0\0\x05\0\0\0world'),
+
+             # strings are sequences in Python, there's no getting away
+             # from it
+#             ('ffff', 'evil', 'e\0v\0i\0l\0'),
+#              ('BBBB', 'evil',                   
+#               '\x01\0\0\0e'
+#               '\x01\0\0\0v'
+#               '\x01\0\0\0i'
+#               '\x01\0\0\0l'),
+
+#              ('', [], ''),
+
+#              # exercise some long strings
+#              ('PP', ['hello' * 255, 'world' * 255],
+#               'hello' * 255 + '\0' + 'world' * 255 + '\0'),
+#              ('PP', ['hello' * 40000, 'world' * 50000],
+#               'hello' * 40000 + '\0' + 'world' * 50000 + '\0'),
+#              ('B', [(5*51), 'hello' * 51], '\xff\0\0\0' + 'hello' * 51),
+#              ('BB', [(5 * 40000), 'hello' * 40000,
+#                      (5 * 50000), 'world' * 50000],
+#               '\x40\x0d\x03\0' + 'hello' * 40000 + '\x90\xd0\x03\x00' + 'world' * 50000),
+
     
 class PackTests(unittest.TestCase):
-    symm_cases = [('B', ['hello' * 51], '\xff\0\0\0' + 'hello' * 51),
+    symm_cases = [
              ('w', [42], '\x2a\0'),
              ('www', [42, 2, 69], '\x2a\0\x02\0\x45\0'),
              ('wd', [42, 256], '\x2a\0\0\x01\0\0'),
@@ -37,55 +68,44 @@ class PackTests(unittest.TestCase):
              ('p', [1], '\x01\0\0\0'),
              ('d', [0x01020304], '\x04\x03\x02\x01'),
              ('d', [0x7fffffff], '\xff\xff\xff\x7f'),
-             ('d', [0x80000000], '\x00\x00\x00\x80'),
-             ('d', [-1], '\xff\xff\xff\xff'),
-             ('d', [-255], '\x01\xff\xff\xff'),
-             ('d', [-256], '\x00\xff\xff\xff'),
+             ('d', [0x80000000L], '\x00\x00\x00\x80'),
+             ('d', [0x80000069L], '\x69\x00\x00\x80'),
+             ('d', [0xffffffffL], '\xff\xff\xff\xff'),
+             ('d', [0xffffff00L], '\x00\xff\xff\xff'),
              ('ddd', [1, 10, 50], '\x01\0\0\0\x0a\0\0\0\x32\0\0\0'),
              ('ff', ['hello', 'world'], 'hello\0world\0'),
              ('fP', ['hello', 'world'], 'hello\0world\0'),
              ('PP', ['hello', 'world'], 'hello\0world\0'),
-             ('B', [''], '\0\0\0\0'),
-             ('B', ['hello'], '\x05\0\0\0hello'),
-             ('BB', ['hello\0world', 'now'],
-              '\x0b\0\0\0hello\0world\x03\0\0\0now'),
-             ('pd', [1, 10], '\x01\0\0\0\x0a\0\0\0'),
-             ('BBB', ['hello', '', 'world'],
-              '\x05\0\0\0hello\0\0\0\0\x05\0\0\0world'),
-
-             # strings are sequences in Python, there's no getting away
-             # from it
-             ('ffff', 'evil', 'e\0v\0i\0l\0'),
-             ('BBBB', 'evil',                   
-              '\x01\0\0\0e'
-              '\x01\0\0\0v'
-              '\x01\0\0\0i'
-              '\x01\0\0\0l'),
-
-             ('', [], ''),
-
-             # exercise some long strings
-             ('PP', ['hello' * 255, 'world' * 255],
-              'hello' * 255 + '\0' + 'world' * 255 + '\0'),
-             ('PP', ['hello' * 40000, 'world' * 50000],
-              'hello' * 40000 + '\0' + 'world' * 50000 + '\0'),
-             ('B', ['hello' * 51], '\xff\0\0\0' + 'hello' * 51),
-             ('BB', ['hello' * 40000, 'world' * 50000],
-              '\x40\x0d\x03\0' + 'hello' * 40000 + '\x90\xd0\x03\x00' + 'world' * 50000),
+             ('B', [0, ''], '\0\0\0\0'),
+# old implementation is wierd when string is not the right length             
+#             ('B', [2, 'hello'], '\x0a\0\0\0hello'),
+             ('B', [5, 'hello'], '\x05\0\0\0hello'),
              ]
 
     def test_symmetric(self):
         """Cookbook of symmetric pack/unpack tests
         """
-        for packer in both_packers:
+        for packer in [samba.tdbpack.pack]: # both_packers:
             for unpacker in both_unpackers:
                 for format, values, expected in self.symm_cases:
-                    self.assertEquals(packer(format, values), expected)
+                    out_packed = packer(format, values)
+                    self.assertEquals(out_packed, expected)
                     out, rest = unpacker(format, expected)
                     self.assertEquals(rest, '')
                     self.assertEquals(list(values), list(out))
-        
-    
+
+    def test_large(self):
+        """Test large pack/unpack strings"""
+        large_cases = [('w' * 1000, xrange(1000)), ]
+        for packer in both_packers:
+            for unpacker in both_unpackers:
+                for format, values in large_cases:
+                    packed = packer(format, values)
+                    out, rest = unpacker(format, packed)
+                    self.assertEquals(rest, '')
+                    self.assertEquals(list(values), list(out))
+
+                    
     def test_pack(self):
         """Cookbook of expected pack values
 
@@ -95,10 +115,6 @@ class PackTests(unittest.TestCase):
         cases = [('w', (42,), '\x2a\0'),
                  ('p', [None], '\0\0\0\0'),
                  ('p', ['true'], '\x01\0\0\0'),
-
-                 ('w', {1: 'fruit'}, '\x01\0'),
-                 # passing a dictionary is dodgy, but it gets coerced to keys
-                 # as if you called list()
                  ]
 
         for packer in both_packers:
@@ -112,7 +128,23 @@ class PackTests(unittest.TestCase):
                 out, rest = unpacker(format, packed + 'hello sailor!')
                 self.assertEquals(rest, 'hello sailor!')
                 self.assertEquals(list(values), list(out))
-        
+
+
+    def test_pack_extra(self):
+        """Leftover values when packing"""
+        cases = [
+            ('d', [10, 20], [10]),
+            ('d', [10, 'hello'], [10]),
+            ('ff', ['hello', 'world', 'sailor'], ['hello', 'world']),
+            ]
+        for unpacker in both_unpackers:
+            for packer in both_packers:
+                for format, values, chopped in cases:
+                    bin = packer(format, values)
+                    out, rest = unpacker(format, bin)
+                    self.assertEquals(list(out), list(chopped))
+                    self.assertEquals(rest, '')
+
 
     def test_unpack(self):
         """Cookbook of tricky unpack tests"""
@@ -129,73 +161,93 @@ class PackTests(unittest.TestCase):
 
     def test_pack_failures(self):
         """Expected errors for incorrect packing"""
-        cases = [('w', [], IndexError),
-                 ('w', (), IndexError),
-                 ('w', {}, IndexError),
-                 ('ww', [2], IndexError),
-                 ('w', 2, TypeError),
-                 ('', [1, 2, 3], IndexError),
-                 ('w', None, TypeError),
-                 ('wwwwwwwwwwww', [], IndexError),
-                 ('w', [2, 3], IndexError),
-                 ('w', [0x60A15EC5L], TypeError),
-                 ('w', [None], TypeError),
-                 ('w', xrange(10000), IndexError),
-                 ('d', [], IndexError),
-                 ('d', [0L], TypeError),
-                 ('p', [], IndexError),
-                 ('f', [2], TypeError),
-                 ('P', [None], TypeError),
-                 ('P', (), IndexError),
-                 ('f', [hex], TypeError),
-                 ('fw', ['hello'], IndexError),
-                 ('f', [u'hello'], TypeError),
-                 ('B', [2], TypeError),
-                 (None, [2, 3, 4], TypeError),
-                 (ord('f'), [20], TypeError),
-                 (['w', 'w'], [2, 2], TypeError),
-                 ('Q', [2], ValueError),
-                 ('fQ', ['2', 3], ValueError),
-                 ('fQ', ['2'], IndexError),
-                 (2, [2], TypeError),
-                 ({}, {}, TypeError)]
+        cases = [('w', []),
+#                 ('w', ()),
+#                 ('w', {}),
+                 ('ww', [2]),
+                 ('w', 2),
+#                  ('w', None),
+                 ('wwwwwwwwwwww', []),
+#                 ('w', [0x60A15EC5L]),
+#                 ('w', [None]),
+                 ('d', []),
+                 ('p', []),
+                 ('f', [2]),
+                 ('P', [None]),
+                 ('P', ()),
+                 ('f', [hex]),
+                 ('fw', ['hello']),
+#                  ('f', [u'hello']),
+                 ('B', [2]),
+                 (None, [2, 3, 4]),
+                 (ord('f'), [20]),
+                 # old code doesn't distinguish string from seq-of-char
+#                 (['w', 'w'], [2, 2]),
+                 # old code just ignores invalid characters
+#                 ('Q', [2]),
+#                 ('fQ', ['2', 3]),
+#                 ('fQ', ['2']),
+                 (2, [2]),
+                 # old code doesn't typecheck format
+#                 ({}, {})
+                 ]
         for packer in both_packers:
-            for format, values, throwable_class in cases:
-                def do_pack():
+            for format, values in cases:
+                try:
                     packer(format, values)
-                self.assertRaises(throwable_class, do_pack)
+                except StandardError:
+                    pass
+                else:
+                    raise AssertionError("didn't get exception: format %s, values %s, packer %s"
+                                         % (`format`, `values`, `packer`))
 
 
     def test_unpack_failures(self):
         """Expected errors for incorrect unpacking"""
-        cases = [('$', '', ValueError),
-                 ('Q', '', ValueError),
-                 ('Q$', '', ValueError),
+        cases = [
+# This ought to be illegal, but the old code doesn't prohibit it
+#                ('$', '', ValueError),
+#                ('Q', '', ValueError),
+#                ('Q$', '', ValueError),
                  ('f', '', IndexError),
                  ('d', '', IndexError),
-                 ('d', '2', IndexError),
-                 ('d', '22', IndexError),
-                 ('d', '222', IndexError),
+# This is an illegal packing, but the old code doesn't trap                 
+#                 ('d', '2', IndexError),
+#                 ('d', '22', IndexError),
+#                 ('d', '222', IndexError),
+#                 ('p', '\x01\0', IndexError),
+#                ('w', '2', IndexError),
+#                ('B', '\xff\0\0\0hello', IndexError),
+#                  ('B', '\xff\0', IndexError),
                  ('w', '', IndexError),
-                 ('w', '2', IndexError),
                  ('f', 'hello', IndexError),
                  ('f', '', IndexError),
-                 ('p', '\x01\0', IndexError),
-                 ('B', '\xff\0\0\0hello', IndexError),
-                 ('B', '\xff\0', IndexError),
-                 ('B', '\x01\0\0\0', IndexError),
-                 ('B', '\x05\0\0\0hell', IndexError),
+#                ('B', '\x01\0\0\0', IndexError),
+#                 ('B', '\x05\0\0\0hell', IndexError),
                  ('B', '\xff\xff\xff\xff', ValueError),
-                 ('B', 'foobar', IndexError),
-                 ('BB', '\x01\0\0\0a\x01', IndexError),
+#                 ('B', 'foobar', IndexError),
+#                 ('BB', '\x01\0\0\0a\x01', IndexError),
                  ]
 
         for unpacker in both_unpackers:
             for format, values, throwable_class in cases:
-                def do_unpack():
+                try:
                     unpacker(format, values)
-            self.assertRaises(throwable_class, do_unpack)
-
+                except StandardError:
+                    pass
+                else:
+                    raise AssertionError("didn't get exception: format %s, values %s, unpacker %s"
+                                         % (`format`, `values`, `unpacker`))
+
+    def test_unpack_repeated(self):
+        cases = [(('df$',
+                  '\x00\x00\x00\x00HP C LaserJet 4500-PS\x00Windows 4.0\x00\\print$\\WIN40\\0\\PSCRIPT.DRV\x00\\print$\\WIN40\\0\\PSCRIPT.DRV\x00\\print$\\WIN40\\0\\PSCRIPT.DRV\x00\\print$\\WIN40\\0\\PSCRIPT.HLP\x00\x00RAW\x00\\print$\\WIN40\\0\\readme.wri\x00\\print$\\WIN40\\0\\pscript.drv\x00\\print$\\WIN40\\0\\pscript.hlp\x00'),
+                 ([0L, 'HP C LaserJet 4500-PS', 'Windows 4.0', '\\print$\\WIN40\\0\\PSCRIPT.DRV', '\\print$\\WIN40\\0\\PSCRIPT.DRV', '\\print$\\WIN40\\0\\PSCRIPT.DRV', '\\print$\\WIN40\\0\\PSCRIPT.HLP', '', 'RAW', '\\print$\\WIN40\\0\\readme.wri', '\\print$\\WIN40\\0\\pscript.drv', '\\print$\\WIN40\\0\\pscript.hlp'], ''))]
+        for unpacker in both_unpackers:
+            for input, expected in cases:
+                result = apply(unpacker, input)
+                if result != expected:
+                    raise AssertionError("%s:\n     input: %s\n    output: %s\n  expected: %s" % (`unpacker`, `input`, `result`, `expected`))
         
 
 if __name__ == '__main__':
diff --git a/source3/python/py_conv.c b/source3/python/py_conv.c
index 20302c83e8..04b4194824 100644
--- a/source3/python/py_conv.c
+++ b/source3/python/py_conv.c
@@ -182,3 +182,27 @@ done:
 
 	return result;
 }
+
+/* Convert a NULL terminated list of NULL terminated unicode strings
+   to a list of (char *) strings */
+
+PyObject *from_unistr_list(uint16 *dependentfiles)
+{
+	PyObject *list;
+	int offset = 0;
+
+	list = PyList_New(0);
+
+	while (*(dependentfiles + offset) != 0) {
+		fstring name;
+		int len;
+
+		len = rpcstr_pull(name, dependentfiles + offset,
+				  sizeof(fstring), -1, STR_TERMINATE);
+
+		offset += len / 2;
+		PyList_Append(list, PyString_FromString(name));
+	}
+
+	return list;
+}
diff --git a/source3/python/py_spoolss.c b/source3/python/py_spoolss.c
index 4451cd87b2..957b4e9d9f 100644
--- a/source3/python/py_spoolss.c
+++ b/source3/python/py_spoolss.c
@@ -297,6 +297,18 @@ Set the form given by the dictionary argument."},
            METH_VARARGS | METH_KEYWORDS,
           "Delete printer data." },
 
+        { "enumprinterkey", (PyCFunction)spoolss_hnd_enumprinterkey,
+           METH_VARARGS | METH_KEYWORDS,
+          "Enumerate printer key." },
+
+#if 0
+        /* Not implemented */
+
+        { "deleteprinterkey", (PyCFunction)spoolss_hnd_deleteprinterkey,
+           METH_VARARGS | METH_KEYWORDS,
+          "Delete printer key." },
+#endif
+
 	{ NULL }
 
 };
diff --git a/source3/python/py_spoolss_drivers_conv.c b/source3/python/py_spoolss_drivers_conv.c
index fd47c0e84d..9bc8408052 100644
--- a/source3/python/py_spoolss_drivers_conv.c
+++ b/source3/python/py_spoolss_drivers_conv.c
@@ -78,30 +78,6 @@ struct pyconv py_DRIVER_DIRECTORY_1[] = {
 	{ NULL }
 };
 
-/* Convert a NULL terminated list of NULL terminated unicode strings
-   to a list of (char *) strings */
-
-static PyObject *from_dependentfiles(uint16 *dependentfiles)
-{
-	PyObject *list;
-	int offset = 0;
-
-	list = PyList_New(0);
-
-	while (*(dependentfiles + offset) != 0) {
-		fstring name;
-		int len;
-
-		len = rpcstr_pull(name, dependentfiles + offset,
-				  sizeof(fstring), -1, STR_TERMINATE);
-
-		offset += len / 2;
-		PyList_Append(list, PyString_FromString(name));
-	}
-
-	return list;
-}
-
 static uint16 *to_dependentfiles(PyObject *dict)
 {
 	return (uint16 *)"abcd\0";
@@ -141,7 +117,7 @@ BOOL py_from_DRIVER_INFO_3(PyObject **dict, DRIVER_INFO_3 *info)
 
 	PyDict_SetItemString(
 		*dict, "dependent_files", 
-		from_dependentfiles(info->dependentfiles));
+		from_unistr_list(info->dependentfiles));
 
 	return True;
 }
@@ -181,7 +157,7 @@ BOOL py_from_DRIVER_INFO_6(PyObject **dict, DRIVER_INFO_6 *info)
 	PyDict_SetItemString(*dict, "level", PyInt_FromLong(6));
 	PyDict_SetItemString(
 		*dict, "dependent_files", 
-		from_dependentfiles (info->dependentfiles));
+		from_unistr_list(info->dependentfiles));
 	return True;
 }
 
diff --git a/source3/python/py_spoolss_printerdata.c b/source3/python/py_spoolss_printerdata.c
index bacc870d9d..583d097e84 100644
--- a/source3/python/py_spoolss_printerdata.c
+++ b/source3/python/py_spoolss_printerdata.c
@@ -391,3 +391,59 @@ PyObject *spoolss_hnd_deleteprinterdataex(PyObject *self, PyObject *args, PyObje
 	Py_INCREF(Py_None);
 	return Py_None;
 }
+
+PyObject *spoolss_hnd_enumprinterkey(PyObject *self, PyObject *args,
+				     PyObject *kw)
+{
+	spoolss_policy_hnd_object *hnd = (spoolss_policy_hnd_object *)self;
+	static char *kwlist[] = { "key", NULL };
+	char *keyname;
+	WERROR werror;
+	uint32 needed, keylist_len;
+	uint16 *keylist;
+	PyObject *result;
+
+	/* Parse parameters */
+
+	if (!PyArg_ParseTupleAndKeywords(args, kw, "s", kwlist, &keyname))
+		return NULL;
+
+	/* Call rpc function */
+
+	werror = cli_spoolss_enumprinterkey(
+		hnd->cli, hnd->mem_ctx, 0, &needed, &hnd->pol,
+		keyname, &keylist, &keylist_len);
+
+	if (W_ERROR_V(werror) == ERRmoredata) 
+		werror = cli_spoolss_enumprinterkey(
+			hnd->cli, hnd->mem_ctx, needed, NULL, &hnd->pol,
+			keyname, &keylist, &keylist_len);
+
+	if (!W_ERROR_IS_OK(werror)) {
+		PyErr_SetObject(spoolss_werror, py_werror_tuple(werror));
+		return NULL;
+	}
+
+	result = from_unistr_list(keylist);
+
+	return result;
+}
+
+#if 0
+
+PyObject *spoolss_hnd_deleteprinterkey(PyObject *self, PyObject *args,
+				       PyObject *kw)
+{
+	spoolss_policy_hnd_object *hnd = (spoolss_policy_hnd_object *)self;
+	static char *kwlist[] = { "key", NULL };
+	char *keyname;
+	WERROR werror;
+
+	if (!PyArg_ParseTupleAndKeywords(args, kw, "s", kwlist, &keyname))
+		return NULL;
+
+	Py_INCREF(Py_None);
+	return Py_None;
+}
+
+#endif
diff --git a/source3/python/py_spoolss_proto.h b/source3/python/py_spoolss_proto.h
index b5c6a3239e..77feb1acc3 100644
--- a/source3/python/py_spoolss_proto.h
+++ b/source3/python/py_spoolss_proto.h
@@ -95,6 +95,10 @@ PyObject *spoolss_hnd_getprinterdataex(PyObject *self, PyObject *args, PyObject
 PyObject *spoolss_hnd_setprinterdataex(PyObject *self, PyObject *args, PyObject *kw);
 PyObject *spoolss_hnd_enumprinterdataex(PyObject *self, PyObject *args, PyObject *kw);
 PyObject *spoolss_hnd_deleteprinterdataex(PyObject *self, PyObject *args, PyObject *kw);
+PyObject *spoolss_hnd_enumprinterkey(PyObject *self, PyObject *args,
+				     PyObject *kw);
+PyObject *spoolss_hnd_deleteprinterkey(PyObject *self, PyObject *args,
+				       PyObject *kw);
 
 /* The following definitions come from python/py_spoolss_printers.c  */
 
diff --git a/source3/python/py_tdbpack.c b/source3/python/py_tdbpack.c
index 06aebe61eb..87cd804ed4 100644
--- a/source3/python/py_tdbpack.c
+++ b/source3/python/py_tdbpack.c
@@ -30,19 +30,19 @@
 static int pytdbpack_calc_reqd_len(char *format_str,
 				   PyObject *val_seq);
 
-static PyObject *pytdbpack_unpack_item(char,
-				      char **pbuf,
-				      int *plen);
-static int
-pytdbpack_calc_item_len(char format_ch,
-			PyObject *val_obj);
+static PyObject *pytdbpack_unpack_item(char, char **pbuf, int *plen, PyObject *);
 
 static PyObject *pytdbpack_pack_data(const char *format_str,
 				     PyObject *val_seq,
 				     unsigned char *buf);
 
 
-	
+
+
+static PyObject *pytdbpack_bad_type(char ch,
+				    const char *expected,
+				    PyObject *val_obj);
+
 static const char * pytdbpack_docstring =
 "Convert between Python values and Samba binary encodings.
 
@@ -67,16 +67,17 @@ tdbpack format strings:
 
     'P':  same as 'f'
 
-    'd':  4 byte little-endian number
+    'd':  4 byte little-endian unsigned number
 
-    'w':  2 byte little-endian number
+    'w':  2 byte little-endian unsigned number
 
     'P': \"Pointer\" value -- in the subset of DCERPC used by Samba, this is
           really just an \"exists\" or \"does not exist\" flag.  The boolean
           value of the Python object is used.
     
     'B': 4-byte LE length, followed by that many bytes of binary data.
-         Corresponds to a Python byte string of the appropriate length.
+         Corresponds to a Python integer giving the length, followed by a byte
+         string of the appropriate length.
 
     '$': Special flag indicating that the preceding format code should be
          repeated while data remains.  This is only supported for unpacking.
@@ -99,12 +100,15 @@ returns:
     buffer -- string containing packed data
 
 raises:
-    IndexError -- if there are not the same number of format codes as of
-        values
+    IndexError -- if there are too few values for the format
     ValueError -- if any of the format characters is illegal
     TypeError  -- if the format is not a string, or values is not a sequence,
         or any of the values is of the wrong type for the corresponding
         format character
+
+notes:
+    For historical reasons, it is not an error to pass more values than are consumed
+    by the format.
 ";
 
 
@@ -205,8 +209,8 @@ pytdbpack_unpack(PyObject *self,
 	PyObject *val_list = NULL, *ret_tuple = NULL;
 	PyObject *rest_string = NULL;
 	int format_len, packed_len;
+	char last_format = '#';	/* invalid */
 	int i;
-	char last_format = '#';
 	
 	/* get arguments */
 	if (!PyArg_ParseTuple(args, "ss#", &format_str, &packed_str, &packed_len))
@@ -214,8 +218,9 @@ pytdbpack_unpack(PyObject *self,
 
 	format_len = strlen(format_str);
 	
-	/* allocate list to hold results */
-	val_list = PyList_New(format_len);
+	/* Allocate list to hold results.  Initially empty, and we append
+	   results as we go along. */
+	val_list = PyList_New(0);
 	if (!val_list)
 		goto failed;
 	ret_tuple = PyTuple_New(2);
@@ -223,34 +228,27 @@ pytdbpack_unpack(PyObject *self,
 		goto failed;
 	
 	/* For every object, unpack.  */
-	for (ppacked = packed_str, i = 0; i < format_len; i++) {
-		PyObject *val_obj;
-		char format;
-
-		format = format_str[i];
-		if (format == '$') {
-			if (i == 0) {
-				PyErr_Format(PyExc_ValueError,
-					     "%s: '$' may not be first character in format",
-					     __FUNCTION__);
-				goto failed;
-			}
-			else {
-				format = last_format; /* repeat */
-			}
-		}
-
-		val_obj = pytdbpack_unpack_item(format,
-						&ppacked,
-						&packed_len);
-		if (!val_obj)
+	for (ppacked = packed_str, i = 0; i < format_len && format_str[i] != '$'; i++) {
+		last_format = format_str[i];
+		/* packed_len is reduced in place */
+		if (!pytdbpack_unpack_item(format_str[i], &ppacked, &packed_len, val_list))
 			goto failed;
-
-		PyList_SET_ITEM(val_list, i, val_obj);
-		last_format = format;
 	}
 
-	/* put leftovers in box for lunch tomorrow */
+	/* If the last character was '$', keep going until out of space */
+	if (format_str[i] == '$') {
+		if (i == 0) {
+			PyErr_Format(PyExc_ValueError,
+				     "%s: '$' may not be first character in format",
+				     __FUNCTION__);
+			return NULL;
+		} 
+		while (packed_len > 0)
+			if (!pytdbpack_unpack_item(last_format, &ppacked, &packed_len, val_list))
+				goto failed;
+	}
+	
+	/* save leftovers for next time */
 	rest_string = PyString_FromStringAndSize(ppacked, packed_len);
 	if (!rest_string)
 		goto failed;
@@ -263,7 +261,8 @@ pytdbpack_unpack(PyObject *self,
 	return ret_tuple;
 
   failed:
-	/* handle failure: deallocate anything */
+	/* handle failure: deallocate anything.  XDECREF forms handle NULL
+	   pointers for objects that haven't been allocated yet. */
 	Py_XDECREF(val_list);
 	Py_XDECREF(ret_tuple);
 	Py_XDECREF(rest_string);
@@ -293,36 +292,71 @@ pytdbpack_calc_reqd_len(char *format_str,
 	int val_i;
 	int val_len;
 
-	val_len = PySequence_Fast_GET_SIZE(val_seq);
+	val_len = PySequence_Length(val_seq);
+	if (val_len == -1)
+		return -1;
 
 	for (p = format_str, val_i = 0; *p; p++, val_i++) {
 		char ch = *p;
-		PyObject *val_obj;
-		int item_len;
 
 		if (val_i >= val_len) {
 			PyErr_Format(PyExc_IndexError,
-				     "samba.tdbpack.pack: value list is too short for format string");
+				     "%s: value list is too short for format string",
+				     __FUNCTION__);
 			return -1;
 		}
 
 		/* borrow a reference to the item */
-		val_obj = PySequence_Fast_GET_ITEM(val_seq, val_i);
-		if (!val_obj)
-			return -1;
+		if (ch == 'd' || ch == 'p') 
+			len += 4;
+		else if (ch == 'w')
+			len += 2;
+		else if (ch == 'f' || ch == 'P') {
+			/* nul-terminated 8-bit string */
+			int item_len;
+			PyObject *str_obj;
+
+			str_obj = PySequence_GetItem(val_seq, val_i);
+			if (!str_obj)
+				return -1;
+
+			if (!PyString_Check(str_obj) || ((item_len = PyString_Size(str_obj)) == -1)) {
+				pytdbpack_bad_type(ch, "String", str_obj);
+				return -1;
+			}
+			
+			len += 1 + item_len;
+		}
+		else if (ch == 'B') {
+			/* length-preceded byte buffer: n bytes, plus a preceding
+			 * word */
+			PyObject *len_obj;
+			long len_val;
+
+			len_obj = PySequence_GetItem(val_seq, val_i);
+			val_i++; /* skip over buffer */
+
+			if (!PyNumber_Check(len_obj)) {
+				pytdbpack_bad_type(ch, "Number", len_obj);
+				return -1;
+			}
 
-		item_len = pytdbpack_calc_item_len(ch, val_obj);
-		if (item_len == -1)
-			return -1;
-		else
-			len += item_len;
-	}
+			len_val = PyInt_AsLong(len_obj);
+			if (len_val < 0) {
+				PyErr_Format(PyExc_ValueError,
+					     "%s: format 'B' requires positive integer", __FUNCTION__);
+				return -1;
+			}
 
-	if (val_i != val_len) {
-		PyErr_Format(PyExc_IndexError,
-			     "%s: value list is wrong length for format string",
-			     __FUNCTION__);
-		return -1;
+			len += 4 + len_val;
+		}
+		else {	
+			PyErr_Format(PyExc_ValueError,
+				     "%s: format character '%c' is not supported",
+				     __FUNCTION__, ch);
+		
+			return -1;
+		}
 	}
 
 	return len;
@@ -345,61 +379,12 @@ static PyObject *pytdbpack_bad_type(char ch,
 
 
 /*
- * Calculate the number of bytes required to pack a single value.  While doing
- * this, also conduct some initial checks that the argument types are
- * reasonable.
- *
- * Returns -1 on exception.
- */
-static int
-pytdbpack_calc_item_len(char ch,
-			PyObject *val_obj)
-{
-	if (ch == 'd' || ch == 'w') {
-		if (!PyInt_Check(val_obj)) {
-			pytdbpack_bad_type(ch, "Int", val_obj);
-			return -1;
-		}
-		if (ch == 'w')
-			return 2;
-		else 
-			return 4;
-	} else if (ch == 'p') {
-		return 4;
-	}
-	else if (ch == 'f' || ch == 'P' || ch == 'B') {
-		/* nul-terminated 8-bit string */
-		if (!PyString_Check(val_obj)) {
-			pytdbpack_bad_type(ch, "String", val_obj);
-		}
-		
-		if (ch == 'B') {
-			/* byte buffer; just use Python string's length, plus
-			   a preceding word */
-			return 4 + PyString_GET_SIZE(val_obj);
-		}
-		else {
-			/* one nul character */
-			return 1 + PyString_GET_SIZE(val_obj);
-		}		
-	}
-	else {	
-		PyErr_Format(PyExc_ValueError,
-			     "tdbpack: format character '%c' is not supported",
-			     ch);
-		
-		return -1;
-	}
-}
-
-
-/*
   XXX: glib and Samba have quicker macro for doing the endianness conversions,
   but I don't know of one in plain libc, and it's probably not a big deal.  I
   realize this is kind of dumb because we'll almost always be on x86, but
   being safe is important.
 */
-static void pack_int32(unsigned long val_long, unsigned char **pbuf)
+static void pack_uint32(unsigned long val_long, unsigned char **pbuf)
 {
 	(*pbuf)[0] =         val_long & 0xff;
 	(*pbuf)[1] = (val_long >> 8)  & 0xff;
@@ -426,9 +411,9 @@ unpack_err_too_short(void)
 
 
 static PyObject *
-unpack_int32(char **pbuf, int *plen)
+unpack_uint32(char **pbuf, int *plen)
 {
-	long v;
+	unsigned long v;
 	unsigned char *b;
 	
 	if (*plen < 4) {
@@ -442,7 +427,7 @@ unpack_int32(char **pbuf, int *plen)
 	(*pbuf) += 4;
 	(*plen) -= 4;
 
-	return PyInt_FromLong(v);
+	return PyLong_FromUnsignedLong(v);
 }
 
 
@@ -490,12 +475,13 @@ unpack_string(char **pbuf, int *plen)
 
 
 static PyObject *
-unpack_buffer(char **pbuf, int *plen)
+unpack_buffer(char **pbuf, int *plen, PyObject *val_list)
 {
 	/* first get 32-bit len */
 	long slen;
 	unsigned char *b;
 	unsigned char *start;
+	PyObject *str_obj = NULL, *len_obj = NULL;
 	
 	if (*plen < 4) {
 		unpack_err_too_short();
@@ -526,7 +512,24 @@ unpack_buffer(char **pbuf, int *plen)
 	(*pbuf) += slen;
 	(*plen) -= slen;
 
-	return PyString_FromStringAndSize(start, slen);
+	if (!(len_obj = PyInt_FromLong(slen)))
+		goto failed;
+
+	if (PyList_Append(val_list, len_obj) == -1)
+		goto failed;
+	
+	if (!(str_obj = PyString_FromStringAndSize(start, slen)))
+		goto failed;
+	
+	if (PyList_Append(val_list, str_obj) == -1)
+		goto failed;
+	
+	return val_list;
+
+  failed:
+	Py_XDECREF(len_obj);	/* handles NULL */
+	Py_XDECREF(str_obj);
+	return NULL;
 }
 
 
@@ -536,24 +539,27 @@ unpack_buffer(char **pbuf, int *plen)
    *PBUF is advanced, and *PLEN reduced to reflect the amount of data that has
    been consumed.
 
-   Returns a reference to the unpacked Python object, or NULL for failure.
+   Returns a reference to None, or NULL for failure.
 */
 static PyObject *pytdbpack_unpack_item(char ch,
 				       char **pbuf,
-				       int *plen)
+				       int *plen,
+				       PyObject *val_list)
 {
+	PyObject *result;
+	
 	if (ch == 'w') {	/* 16-bit int */
-		return unpack_int16(pbuf, plen);
+		result = unpack_int16(pbuf, plen);
 	}
 	else if (ch == 'd' || ch == 'p') { /* 32-bit int */
 		/* pointers can just come through as integers */
-		return unpack_int32(pbuf, plen);
+		result = unpack_uint32(pbuf, plen);
 	}
 	else if (ch == 'f' || ch == 'P') { /* nul-term string  */
-		return unpack_string(pbuf, plen);
+		result = unpack_string(pbuf, plen);
 	}
 	else if (ch == 'B') { /* length, buffer */
-		return unpack_buffer(pbuf, plen);
+		return unpack_buffer(pbuf, plen, val_list);
 	}
 	else {
 		PyErr_Format(PyExc_ValueError,
@@ -562,68 +568,19 @@ static PyObject *pytdbpack_unpack_item(char ch,
 		
 		return NULL;
 	}
-}
-
-
-
-/*
-  Pack a single item VAL_OBJ, encoded using format CH, into a buffer at *PBUF,
-  and advance the pointer.  Buffer length has been pre-calculated so we are
-  sure that there is enough space.
-
-*/
-static PyObject *
-pytdbpack_pack_item(char ch,
-		    PyObject *val_obj,
-		    unsigned char **pbuf)
-{
-	if (ch == 'w') {
-		unsigned long val_long = PyInt_AsLong(val_obj);
-		(*pbuf)[0] = val_long & 0xff;
-		(*pbuf)[1] = (val_long >> 8) & 0xff;
-		(*pbuf) += 2;
-	}
-	else if (ch == 'd') {
-		/* 4-byte LE number */
-		pack_int32(PyInt_AsLong(val_obj), pbuf);
-	}
-	else if (ch == 'p') {
-		/* "Pointer" value -- in the subset of DCERPC used by Samba,
-		   this is really just an "exists" or "does not exist"
-		   flag. */
-		pack_int32(PyObject_IsTrue(val_obj), pbuf);
-	}
-	else if (ch == 'f' || ch == 'P') {
-		int size;
-		char *sval;
 
-		size = PyString_GET_SIZE(val_obj);
-		sval = PyString_AS_STRING(val_obj);
-		pack_bytes(size+1, sval, pbuf); /* include nul */
-	}
-	else if (ch == 'B') {
-		int size;
-		char *sval;
-
-		size = PyString_GET_SIZE(val_obj);
-		pack_int32(size, pbuf);
-		sval = PyString_AS_STRING(val_obj);
-		pack_bytes(size, sval, pbuf); /* do not include nul */
-	}
-	else {
-		/* this ought to be caught while calculating the length, but
-		   just in case. */
-		PyErr_Format(PyExc_ValueError,
-			     "%s: format character '%c' is not supported",
-			     __FUNCTION__, ch);
-		
+	/* otherwise OK */
+	if (!result)
 		return NULL;
-	}
-		
-	return Py_None;
+	if (PyList_Append(val_list, result) == -1)
+		return NULL;
+	
+	return val_list;
 }
 
 
+
+
 /*
   Pack data according to FORMAT_STR from the elements of VAL_SEQ into
   PACKED_BUF.
@@ -639,30 +596,109 @@ pytdbpack_pack_item(char ch,
 PyObject *
 pytdbpack_pack_data(const char *format_str,
 		    PyObject *val_seq,
-		    unsigned char *packed_buf)
+		    unsigned char *packed)
 {
-	int i;
+	int format_i, val_i = 0;
 
-	for (i = 0; format_str[i]; i++) {
-		char ch = format_str[i];
+	for (format_i = 0, val_i = 0; format_str[format_i]; format_i++) {
+		char ch = format_str[format_i];
 		PyObject *val_obj;
 
 		/* borrow a reference to the item */
-		val_obj = PySequence_Fast_GET_ITEM(val_seq, i);
+		val_obj = PySequence_GetItem(val_seq, val_i++);
 		if (!val_obj)
 			return NULL;
 
-		if (!pytdbpack_pack_item(ch, val_obj, &packed_buf))
+		if (ch == 'w') {
+			unsigned long val_long;
+			PyObject *long_obj;
+			
+			if (!(long_obj = PyNumber_Long(val_obj))) {
+				pytdbpack_bad_type(ch, "Long", val_obj);
+				return NULL;
+			}
+			
+			val_long = PyLong_AsUnsignedLong(long_obj);
+			(packed)[0] = val_long & 0xff;
+			(packed)[1] = (val_long >> 8) & 0xff;
+			(packed) += 2;
+			Py_DECREF(long_obj);
+		}
+		else if (ch == 'd') {
+			/* 4-byte LE number */
+			PyObject *long_obj;
+			
+			if (!(long_obj = PyNumber_Long(val_obj))) {
+				pytdbpack_bad_type(ch, "Long", val_obj);
+				return NULL;
+			}
+			
+			pack_uint32(PyLong_AsUnsignedLong(long_obj), &packed);
+
+			Py_DECREF(long_obj);
+		}
+		else if (ch == 'p') {
+			/* "Pointer" value -- in the subset of DCERPC used by Samba,
+			   this is really just an "exists" or "does not exist"
+			   flag. */
+			pack_uint32(PyObject_IsTrue(val_obj), &packed);
+		}
+		else if (ch == 'f' || ch == 'P') {
+			int size;
+			char *sval;
+
+			size = PySequence_Length(val_obj);
+			if (size < 0)
+				return NULL;
+			sval = PyString_AsString(val_obj);
+			if (!sval)
+				return NULL;
+			pack_bytes(size+1, sval, &packed); /* include nul */
+		}
+		else if (ch == 'B') {
+			long size;
+			char *sval;
+
+			if (!PyNumber_Check(val_obj)) {
+				pytdbpack_bad_type(ch, "Number", val_obj);
+				return NULL;
+			}
+
+			if (!(val_obj = PyNumber_Long(val_obj)))
+				return NULL;
+
+			size = PyLong_AsLong(val_obj);
+			pack_uint32(size, &packed);
+
+			/* Release the new reference created by the cast */
+			Py_DECREF(val_obj);
+
+			val_obj = PySequence_GetItem(val_seq, val_i++);
+			if (!val_obj)
+				return NULL;
+			
+			sval = PyString_AsString(val_obj);
+			if (!sval)
+				return NULL;
+			
+			pack_bytes(size, sval, &packed); /* do not include nul */
+		}
+		else {
+			/* this ought to be caught while calculating the length, but
+			   just in case. */
+			PyErr_Format(PyExc_ValueError,
+				     "%s: format character '%c' is not supported",
+				     __FUNCTION__, ch);
+		
 			return NULL;
+		}
 	}
-
+		
 	return Py_None;
 }
 
 
 
-
-
 static PyMethodDef pytdbpack_methods[] = {
 	{ "pack", pytdbpack_pack, METH_VARARGS, (char *) pytdbpack_pack_doc },
 	{ "unpack", pytdbpack_unpack, METH_VARARGS, (char *) pytdbpack_unpack_doc },