yuja added a comment.
> > Perhaps the easiest way around is to convert an internal nodetree back
to
> > a plain C struct.
>
> You mean to embed that plain C struct in the `parsers.index` Python type
and in the `parsers.nodetree` type as you suggested earlier? Yes, that's
> > Perhaps the easiest way around is to convert an internal nodetree back to
> > a plain C struct.
>
> You mean to embed that plain C struct in the `parsers.index` Python type
> and in the `parsers.nodetree` type as you suggested earlier? Yes, that's
> probably our best option. I'll
martinvonz added a comment.
In https://phab.mercurial-scm.org/D4118#66834, @yuja wrote:
> > static int nt_init(nodetree *self, indexObject *index, unsigned
capacity)
> > {
> >
> > + /* Initialize before argument-checking to avoid nt_dealloc() crash. */
> > +
> static int nt_init(nodetree *self, indexObject *index, unsigned capacity)
> {
> + /* Initialize before argument-checking to avoid nt_dealloc() crash. */
> + self->nodes = NULL;
> +
> self->index = index;
> + Py_INCREF(index);
While thinking about a pure nodetree, I noticed
yuja added a comment.
> static int nt_init(nodetree *self, indexObject *index, unsigned capacity)
> {
>
> + /* Initialize before argument-checking to avoid nt_dealloc() crash. */
> + self->nodes = NULL;
> +
>
> self->index = index;
>
> + Py_INCREF(index);
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGb85b377e7fc2: index: make node tree a Python object
(authored by martinvonz, committed by ).
CHANGED PRIOR TO COMMIT
https://phab.mercurial-scm.org/D4118?vs=10456=10482#toc
REPOSITORY
rHG Mercurial
> static int nt_init(nodetree *self, indexObject *index, unsigned capacity)
> {
> + /* Initialize before argument-checking to avoid nt_dealloc() crash. */
> + self->nodes = NULL;
This comment seems a bit confusing since another argument-checking is done
before `nt_init_py()`.
>
yuja added a comment.
> static int nt_init(nodetree *self, indexObject *index, unsigned capacity)
> {
>
> + /* Initialize before argument-checking to avoid nt_dealloc() crash. */
> + self->nodes = NULL;
This comment seems a bit confusing since another argument-checking is
martinvonz updated this revision to Diff 10456.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4118?vs=10272=10456
REVISION DETAIL
https://phab.mercurial-scm.org/D4118
AFFECTED FILES
mercurial/cext/parsers.c
mercurial/cext/revlog.c
martinvonz updated this revision to Diff 10272.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4118?vs=10093=10272
REVISION DETAIL
https://phab.mercurial-scm.org/D4118
AFFECTED FILES
mercurial/cext/parsers.c
mercurial/cext/revlog.c
martinvonz added a comment.
In https://phab.mercurial-scm.org/D4118#64505, @yuja wrote:
> > > I think the default tp_new clears the memory of the object, so it
won't be uninitialized (maybe that's what you mean by "luckily", but I first
thought your meant it would require luck for
> > I think the default tp_new clears the memory of the object, so it won't
> be uninitialized (maybe that's what you mean by "luckily", but I first
> thought your meant it would require luck for it to be set to 0).
>
> Interesting. tp_alloc is documented to initialize memory to zeros.
yuja added a comment.
> > I think the default tp_new clears the memory of the object, so it
won't be uninitialized (maybe that's what you mean by "luckily", but I first
thought your meant it would require luck for it to be set to 0).
>
> Interesting. tp_alloc is documented to
> +static int nt_init_py(nodetree *self, PyObject *args)
> +{
> + PyObject *index;
> + unsigned capacity;
> + if (!PyArg_ParseTuple(args, "OI", , ))
"O!I" to make sure index is an index object.
___
Mercurial-devel mailing list
yuja added a comment.
> +static int nt_init_py(nodetree *self, PyObject *args)
> +{
> + PyObject *index;
> + unsigned capacity;
> + if (!PyArg_ParseTuple(args, "OI", , ))
"O!I" to make sure index is an index object.
REPOSITORY
rHG Mercurial
REVISION DETAIL
yuja added a comment.
> I think the default tp_new clears the memory of the object, so it won't
be uninitialized (maybe that's what you mean by "luckily", but I first thought
your meant it would require luck for it to be set to 0).
Interesting. tp_alloc is documented to initialize
> I think the default tp_new clears the memory of the object, so it won't be
> uninitialized (maybe that's what you mean by "luckily", but I first thought
> your meant it would require luck for it to be set to 0).
Interesting. tp_alloc is documented to initialize memory to zeros.
martinvonz updated this revision to Diff 10093.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4118?vs=10082=10093
REVISION DETAIL
https://phab.mercurial-scm.org/D4118
AFFECTED FILES
mercurial/cext/parsers.c
mercurial/cext/revlog.c
martinvonz updated this revision to Diff 10082.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4118?vs=10042=10082
REVISION DETAIL
https://phab.mercurial-scm.org/D4118
AFFECTED FILES
mercurial/cext/parsers.c
mercurial/cext/revlog.c
martinvonz added a comment.
In https://phab.mercurial-scm.org/D4118#64261, @yuja wrote:
> Can you bump the cext version as this patch introduces new API?
Done. Thanks for the reminder.
>
>
>> +static int nt_init_py(nodetree *self, PyObject *args)
>> +{
>> +
yuja added a comment.
> + PyObject *index;
> + unsigned capacity;
> + if (!PyArg_ParseTuple(args, "OI", , ))
> + return -1;
> + return nt_init(self, (indexObject*)index, capacity);
One more thing, we'll probably need to enforce the index type, by `"O!"`.
And
> + PyObject *index;
> + unsigned capacity;
> + if (!PyArg_ParseTuple(args, "OI", , ))
> + return -1;
> + return nt_init(self, (indexObject*)index, capacity);
One more thing, we'll probably need to enforce the index type, by `"O!"`.
And a lookup of `0xxx...` hash
Can you bump the cext version as this patch introduces new API?
> +static int nt_init_py(nodetree *self, PyObject *args)
> +{
> + PyObject *index;
> + unsigned capacity;
> + if (!PyArg_ParseTuple(args, "OI", , ))
> + return -1;
It leaves self->nodes uninitialized on
yuja added a comment.
Can you bump the cext version as this patch introduces new API?
> +static int nt_init_py(nodetree *self, PyObject *args)
> +{
> + PyObject *index;
> + unsigned capacity;
> + if (!PyArg_ParseTuple(args, "OI", , ))
> + return -1;
It
martinvonz updated this revision to Diff 10042.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4118?vs=9983=10042
REVISION DETAIL
https://phab.mercurial-scm.org/D4118
AFFECTED FILES
mercurial/cext/revlog.c
CHANGE DETAILS
diff --git
martinvonz updated this revision to Diff 9983.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4118?vs=9924=9983
REVISION DETAIL
https://phab.mercurial-scm.org/D4118
AFFECTED FILES
mercurial/cext/revlog.c
CHANGE DETAILS
diff --git
martinvonz updated this revision to Diff 9924.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D4118?vs=9921=9924
REVISION DETAIL
https://phab.mercurial-scm.org/D4118
AFFECTED FILES
mercurial/cext/revlog.c
CHANGE DETAILS
diff --git
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D4118
AFFECTED FILES
mercurial/cext/revlog.c
CHANGE DETAILS
diff --git a/mercurial/cext/revlog.c
28 matches
Mail list logo
