local memory contexts to avoid a discard_const_p()

Matthias Dieter Wallnöfer mdw at samba.org
Mon Dec 13 03:55:50 MST 2010


Hi Andrew,

but is there some particular reason that "oid" from 
"dsdb_schema_pfm_oid_from_attid" is returned "const" - as far as I've seen it's 
an allocated thing? If it's not then we would not need "discard_const_p" at all.

Cheers,
Matthias


On Sun, 2010-12-12 at 21:11 +0100, Jelmer Vernooij wrote:

> Hi Matthias,
> 
> As discussed earlier, can you please send this kind of patch to the list
> for review first? 
> 
> I have my doubts about the usefulness of these changes in general
> compared to the benefit. Eliminating talloc contexts doesn't really gain
> us much. The overhead of creating an extra talloc context compared to
> the overhead of using Python in the first place is negligible.

> > @@ -248,27 +253,19 @@ static PyObject *py_dsdb_get_oid_from_attid(PyObject 
>*self, PyObject *args)
> >  
> >      PyErr_LDB_OR_RAISE(py_ldb, ldb);
> >  
> > -    mem_ctx = talloc_new(NULL);
> > -    if (mem_ctx == NULL) {
> > -       PyErr_NoMemory();
> > -       return NULL;
> > -    }
> > -
> >      schema = dsdb_get_schema(ldb, NULL);
> >  
> >      if (!schema) {
> >          PyErr_SetString(PyExc_RuntimeError, "Failed to find a schema from 
>ldb \n");
> > -        talloc_free(mem_ctx);
> >          return NULL;
> >      }
> >      
> >      status = dsdb_schema_pfm_oid_from_attid(schema->prefixmap, attid,
> > -                                            mem_ctx, &oid);
> > +                                            NULL, &oid);
> >      PyErr_WERROR_IS_ERR_RAISE(status);
> >  
> >      ret = PyString_FromString(oid);
> > -
> > -    talloc_free(mem_ctx);
> > +    talloc_free(discard_const_p(char, oid));
> ^^ Is this really necessary? I'd rather have the extra memory context
> than add an extra discard_const_p.

I agree, it's really useful to be able to avoid the const issues by
using a new memory context here.  Every discard_const_p() hides a
potentially serious bug, and so should be avoided where other reasonable
alternatives exist).  The local memory context seems a very reasonable
alternative, and so it seems a backwards step to eliminate it.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team          http://samba.org
Samba Developer, Cisco Inc.




More information about the samba-technical mailing list