D2939: fsmonitor: layer on another hack in bser.c for os.stat() compat (issue5811)

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Wed Mar 28 20:15:30 EDT 2018


indygreg added a comment.


  I managed to debug this.
  
  `st[state.ST_MTIME]` will invoke `PyObject_GetItem`. Its implementation does this:
  
    m = o->ob_type->tp_as_mapping;
    if (m && m->mp_subscript)
        return m->mp_subscript(o, key);
    
    if (o->ob_type->tp_as_sequence) {
        if (PyIndex_Check(key)) {
            Py_ssize_t key_value;
            key_value = PyNumber_AsSsize_t(key, PyExc_IndexError);
            if (key_value == -1 && PyErr_Occurred())
                return NULL;
            return PySequence_GetItem(o, key_value);
        }
  
  Our `bserObjectType` defines `tp_as_mapping` and `st[state.ST_MTIME]` ends up calling `bserobj_getattrro()`. That function sees the argument is an integer and ends up calling `PySequence_GetItem()`:
  
    if (PyIndex_Check(name)) {
      i = PyNumber_AsSsize_t(name, PyExc_IndexError);
      if (i == -1 && PyErr_Occurred()) {
        goto bail;
      }
      ret = PySequence_GetItem(obj->values, i);
      goto bail;
    }
  
  `PySequence_GetItem()` does the following:
  
    m = s->ob_type->tp_as_sequence;
    if (m && m->sq_item) {
        if (i < 0) {
            if (m->sq_length) {
                Py_ssize_t l = (*m->sq_length)(s);
                if (l < 0)
                    return NULL;
                i += l;
            }
        }
        return m->sq_item(s, i);
    }
  
  `sq_item` here is `tupleitem()`, not `bserobj_tuple_item()`. It tells the truth and `IndexError` is raised.
  
  So, I think we want the hack to live in `bserobj_getattrro()`. I don't think the hack needs to live in `bserobj_tuple_item()` because that function only ever gets called through `bserobj_getattrro()`.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D2939

To: durin42, #hg-reviewers, indygreg
Cc: spectral, indygreg, mercurial-devel


More information about the Mercurial-devel mailing list