This is an archive of the discontinued Mercurial Phabricator instance.

index: extract a type for the nodetree
ClosedPublic

Authored by martinvonz on Aug 5 2018, 3:58 AM.

Details

Summary

This is a first step towards exposing the nodetree as a Python type.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Aug 5 2018, 3:58 AM
yuja added a subscriber: yuja.Aug 6 2018, 9:10 AM

Queued, but there seem to be a couple of minor errors. Can you fix them
if they remain in the code after the refactor?

static int nt_new(indexObject *self)
{

	if (self->ntlength == self->ntcapacity) {
  • if (self->ntcapacity >= INT_MAX / (sizeof(nodetree) * 2)) {

+ if (self->ntcapacity >= INT_MAX / (sizeof(nodetreenode) * 2)) {

			PyErr_SetString(PyExc_MemoryError,
					"overflow in nt_new");
			return -1;
		}
		self->ntcapacity *= 2;
  • self->nt = realloc(self->nt,
  • self->ntcapacity * sizeof(nodetree));
  • if (self->nt == NULL) {

+ self->nt->nodes = realloc(self->nt->nodes,
+ self->ntcapacity * sizeof(nodetreenode));
+ if (self->nt->nodes == NULL) {

			PyErr_SetString(PyExc_MemoryError, "out of memory");
			return -1;

Need to free and nullify self->nt here?
Leaving self->nt && !self->nt->nodes state will probably lead to SEVG.

Another option is to leave ntcapacity and nodes unmodified on realloc
failure.

static int nt_init(indexObject *self)
{

	if (self->nt == NULL) {
  • if ((size_t)self->raw_length > INT_MAX / sizeof(nodetree)) {

+ self->nt = PyMem_Malloc(sizeof(nodetree));

PyMem_Malloc() and PyMem_Free() should be paired.

+ if ((size_t)self->raw_length > INT_MAX / sizeof(nodetreenode)) {

			PyErr_SetString(PyExc_ValueError, "overflow in nt_init");

Need to free and nullify self->nt here?

This revision was automatically updated to reflect the committed changes.

For the record, I'm not thrilled about queuing C code with known bugs.

yuja added a comment.Aug 7 2018, 10:03 AM
For the record, I'm not thrilled about queuing C code with known bugs.

Me neither. If there weren't unmanageable number of patches, I wouldn't
queue.

The fixes are pushed as 05c1f5f49ebb, f7d8fb2ed8a8, and dcd395dc98d8. Thanks.