Sync up python stuff from HEAD.
authorTim Potter <tpot@samba.org>
Thu, 7 Nov 2002 01:12:24 +0000 (01:12 +0000)
committerTim Potter <tpot@samba.org>
Thu, 7 Nov 2002 01:12:24 +0000 (01:12 +0000)
source/python/examples/tdbpack/test_tdbpack.py
source/python/py_conv.c
source/python/py_spoolss.c
source/python/py_spoolss_drivers_conv.c
source/python/py_spoolss_printerdata.c
source/python/py_spoolss_proto.h
source/python/py_tdbpack.c

index 659dc0efed98019c86cdcd439ee9eaee652041ac..83282e745e40ca3dd3c6a57ec9e0dda9e16a6a07 100755 (executable)
@@ -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__':
index 20302c83e87045424d3e854939fffb6302ca7d8e..04b41948240009b95c1c0b5e971a4003d077b50b 100644 (file)
@@ -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;
+}
index 4451cd87b2a80bbf1722061a05ce74a4ce1861ed..957b4e9d9f114d18fd056f6ae6b1c48a2b791e28 100644 (file)
@@ -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 }
 
 };
index fd47c0e84da5d502b345c84684569f23f62b603a..9bc84080529eff6a72f717af9814592ae0761681 100644 (file)
@@ -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;
 }
 
index bacc870d9dd6b86af20aaa26de010a012ff9d260..583d097e845a372d1cd13e5129a66d71970341da 100644 (file)
@@ -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
index b5c6a3239ed510533e15fbfee1d3db620eea94d7..77feb1acc30da563407ebba8026d5010acf0e8ac 100644 (file)
@@ -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  */
 
index 06aebe61eb5ede6e9d95ae09a8ded31041c18ece..87cd804ed4ef14c1b457501f8cd85be063ae1cef 100644 (file)
 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;
@@ -344,62 +378,13 @@ 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 },