Comment 12 for bug 1899193

Revision history for this message
Seth Arnold (seth-arnold) wrote :

Hello Julian, the file descriptor leak patch has a section that I'm not sure about:

 static PyObject *debfile_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 {
- PyDebFileObject *self = (PyDebFileObject*)ararchive_new(type, args, kwds);
+ PyApt_UniqueObject<PyDebFileObject> self((PyDebFileObject*)ararchive_new(type, args, kwds));
     if (self == NULL)
         return NULL;

     // DebFile
- self->control = debfile_get_tar(self, "control.tar");
+ self->control = debfile_get_tar(self.get(), "control.tar");
     if (self->control == NULL)
         return NULL;

- self->data = debfile_get_tar(self, "data.tar");
+ self->data = debfile_get_tar(self.get(), "data.tar");
     if (self->data == NULL)
         return NULL;

@@ -603,7 +603,7 @@ static PyObject *debfile_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
     self->Fd.Read(value, member->Size, true);
     self->debian_binary = PyBytes_FromStringAndSize(value, member->Size);
     delete[] value;
- return self;
+ return self.release();
 }

What happens with those return NULL cases if the data.tar or control.tar entries don't exist? Is that a new leak? I expected a self.release() before those cases.

(Or, alternatively, I don't understand the 'return self.release()' idiom.)

There's also a minor typo "file descritor" in the header. Do what you wish with this.

Thanks