local memory contexts to avoid a discard_const_p()

[Jelmer Vernooij]

On Mon, 2010-12-13 at 10:55 +0000, Matthias Dieter Wallnöfer wrote:

> 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.
No, that indeed doesn't seem necessary. At first glance, changing that
function to return a non-const seems like a more reasonable approach. It
seems like it would also require changing some of the ber_read_*
functions though.

Cheers,

Jelmer
> 
> 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.
> 
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20101213/a71e3b42/attachment.pgp>